Implement FilterChainMatch algorithm #25757

yashykt · 2021-03-18T13:07:37Z

This change is

markdroth

This looks like a great start!

My only really substantive comments are about how we're returning the filter chain info in the LdsResponse struct; it seems like we're actually returning the raw form (a list of FilterChains) in addition to the new struct, but that raw form is never going to be used for anything else, so it seems a little pointless to have it there.

Please let me know if you have any questions. Thanks!

Reviewed 8 of 16 files at r1, 4 of 6 files at r2, 2 of 4 files at r3, 4 of 4 files at r4.
Reviewable status: all files reviewed, 55 unresolved discussions (waiting on @yashykt)

src/core/ext/transport/chttp2/server/chttp2_server.cc, line 97 at r4 (raw file):

    void UpdateConnectionManager(
        RefCountedPtr<grpc_server_config_fetcher::ConnectionManager> manager)

Please call this argument connection_manager.

src/core/ext/transport/chttp2/server/chttp2_server.cc, line 721 at r4 (raw file):

      grpc_error* error = GRPC_ERROR_CREATE_FROM_STATIC_STRING(
          "No ConnectionManager configured. Closing connection.");
      endpoint_cleanup(error);

Need to return after this.

src/core/ext/transport/chttp2/server/chttp2_server.cc, line 721 at r4 (raw file):

      grpc_error* error = GRPC_ERROR_CREATE_FROM_STATIC_STRING(
          "No ConnectionManager configured. Closing connection.");
      endpoint_cleanup(error);

I think this also needs to destroy args.

src/core/ext/transport/chttp2/server/chttp2_server.cc, line 723 at r4 (raw file):

      endpoint_cleanup(error);
    }
    // TODO(yashykt): Maybe combine the following two arg modifiers into a

Unfortunately, doesn't look like there's an easy way to do this. Although the second one will probably go away once insecure builds are gone.

src/core/ext/xds/xds_api.h, line 341 at r2 (raw file):

    };

    using SourcePortsMap =

It might be helpful to have an overall comment here indicating the high-level view of this data structure. Something like this (if I understand correctly):

A multi-level map used to determine which filter chain to use for a given incoming connection.
Determining the right filter chain for a given connection checks the following properties, in order:
- destination port (never matched, so not present in map)
- destination IP range
- server name (never matched, so not present in map)
- transport protocol (allows only "raw_buffer" or unset, prefers the former)
- application protocol (never matched, so not present in map)
- connection source type
- source IP
- source port

src/core/ext/xds/xds_api.h, line 354 at r2 (raw file):

    };
    using SourceIpMap = std::map<std::string, SourceIp>;
    using SourceTypesArray = std::array<SourceIpMap, 3>;

Suggest calling this ConnectionSourceTypesArray, to better match the enum type name used in FilterChainMatch above.

src/core/ext/xds/xds_api.h, line 383 at r2 (raw file):

    // host:port listening_address set when type is kTcpListener
    std::string address;
    std::vector<FilterChain> filter_chains;

Do we still need this field if we're returning destination_ip_map? Doesn't that field contain the same thing, and won't the code that uses these fields only look at that one?

src/core/ext/xds/xds_api.h, line 346 at r4 (raw file):

    };

    struct FilterChainDataSharedPtr {

Having a separate struct just to hold a shared_ptr seems a little cumbersome. It looks like the reason it's here is to allow comparisons for the overall struct to work correctly. But I wonder if we can avoid this by not using a shared_ptr to begin with. I'm thinking that we could instead do something like this:

struct SourcePortsMap {
  // Stores the data entries.
  std::vector<FilterChainData> storage;
  // Map from ports to entries in the storage field.
  std::map<uint16_t, FilterChainData*> map;
};

This both cleans up this struct and avoids the locking overhead of shared_ptr.

src/core/ext/xds/xds_api.cc, line 2246 at r4 (raw file):

                                           ports_map, 0);
  } else {
    for (const auto& port : filter_chain_match.source_ports) {

Please change auto& to uint32_t.

src/core/ext/xds/xds_api.cc, line 2310 at r4 (raw file):

  return AddFilterChainDataForSourceIpRange(
      filter_chain_match, std::move(filter_chain_data),
      &destination_ip->source_types_array[static_cast<int>(

Is the cast actually needed here?

src/core/ext/xds/xds_api.cc, line 2332 at r4 (raw file):

        filter_chain_data,
    XdsApi::LdsUpdate::DestinationIp* destination_ip) {
  auto& transport_protocol = filter_chain_match.transport_protocol;

std::string&

src/core/ext/xds/xds_server_config_fetcher.cc, line 70 at r4 (raw file):

 private:
  struct CertificateProviders {
    RefCountedPtr<grpc_tls_certificate_provider> root;

Why do we need to save our own refs to the root and instance providers? Doesn't XdsCertificateProvider already internally hold refs to those?

src/core/ext/xds/xds_server_config_fetcher.cc, line 114 at r4 (raw file):

  int port = 0;
  if (!absl::SimpleAtoi(port_str, &port)) return nullptr;
  const auto& match = source_ports_map.find(port);

This should not be a reference, and const is unnecessary. Using a const reference here winds up triggering case 8 as described in https://abseil.io/tips/101. For more details, see https://abseil.io/tips/107.

src/core/ext/xds/xds_server_config_fetcher.cc, line 114 at r4 (raw file):

  int port = 0;
  if (!absl::SimpleAtoi(port_str, &port)) return nullptr;
  const auto& match = source_ports_map.find(port);

Suggest calling this it to represent the fact that it's an iterator.

src/core/ext/xds/xds_server_config_fetcher.cc, line 119 at r4 (raw file):

  }
  // Search for the catch-all port 0 since we didn't get a direct match
  const auto& catch_all_match = source_ports_map.find(0);

Same as above. Suggest just reusing the previous it variable here.

src/core/ext/xds/xds_server_config_fetcher.cc, line 157 at r4 (raw file):

    grpc_endpoint* tcp, absl::string_view destination_ip) {
  auto source_uri = URI::Parse(grpc_endpoint_get_peer(tcp));
  if (source_uri->scheme() != "ipv4" && source_uri->scheme() != "ipv6") {

Need to check that source_uri.ok() is true first.

src/core/ext/xds/xds_server_config_fetcher.cc, line 162 at r4 (raw file):

  std::string host;
  std::string port;
  if (!SplitHostPort(source_uri->path(), &host, &port)) {

How about just using grpc_parse_uri() here?

src/core/ext/xds/xds_server_config_fetcher.cc, line 169 at r4 (raw file):

                          0 /* port doesn't matter here */);
  // Use kAny only if kSameIporLoopback and kExternal are empty
  if (source_types_array[static_cast<int>(

Are the casts actually needed here?

src/core/ext/xds/xds_server_config_fetcher.cc, line 203 at r4 (raw file):

    grpc_endpoint* tcp) {
  auto destination_uri = URI::Parse(grpc_endpoint_get_local_address(tcp));
  if (destination_uri->scheme() != "ipv4" &&

Need to first check that destination_uri.ok() is true.

src/core/ext/xds/xds_server_config_fetcher.cc, line 209 at r4 (raw file):

  std::string host;
  std::string port;
  if (!SplitHostPort(destination_uri->path(), &host, &port)) {

How about just using grpc_parse_uri() here?

src/core/ext/xds/xds_server_config_fetcher.cc, line 307 at r4 (raw file):

  const auto* filter_chain =
      FindFilterChainDataForDestinationIp(destination_ip_map_, tcp);
  if (filter_chain == nullptr && default_filter_chain_) {

default_filter_chain_ != nullptr

src/core/ext/xds/xds_server_config_fetcher.cc, line 320 at r4 (raw file):

  if (server_creds == nullptr || server_creds->type() != kCredentialsTypeXds) {
    grpc_channel_args* updated_args =
        grpc_channel_args_copy_and_remove(args, args_to_remove, 1);

Why are we removing the credentials from args if they're not XdsCreds? Won't this break the ability to use xDS in the server without using XdsCreds? I think we still want to support that.

Let's make sure we have a test for this case.

src/core/ext/xds/xds_server_config_fetcher.cc, line 330 at r4 (raw file):

    return result.status();
  }
  RefCountedPtr<XdsCertificateProvider> xds_certificate_provider = *result;

std::move()

src/core/ext/xds/xds_server_config_fetcher.cc, line 431 at r4 (raw file):

        }
      }
      if (filter_chain_match_manager_ == nullptr ||

I think this can be written a little more clearly by inverting the second condition:

if (filter_chain_match_manager_ == nullptr ||
    listener.destination_ip_map !=
        filter_chain_match_manager_->destination_ip_map() ||
    listener.default_filter_chain !=
        filter_chain_match_manager_->default_filter_chain()) {

src/core/lib/gprpp/atomic.h, line 84 at r4 (raw file):

  // Atomically increment a counter only if the counter value is not zero.
  // Returns true if increment took place; false if counter is zero.
  bool IncrementIfNonzero() {

Why was this change necessary?

src/core/lib/iomgr/sockaddr_utils.h, line 83 at r2 (raw file):

    const grpc_resolved_address* resolved_addr);

void grpc_sockaddr_mask_bits(grpc_resolved_address* address,

Please document this function.

src/core/lib/iomgr/sockaddr_utils.h, line 86 at r2 (raw file):

                             uint32_t mask_bits);

bool grpc_sockaddr_match_subnet(const grpc_resolved_address* address,

Same here.

src/core/lib/iomgr/sockaddr_utils.cc, line 383 at r2 (raw file):

    auto* addr4 = reinterpret_cast<const grpc_sockaddr_in*>(addr);
    auto* subnet_addr4 = reinterpret_cast<const grpc_sockaddr_in*>(subnet_addr);
    if (memcmp(&addr4->sin_addr, &subnet_addr4->sin_addr,

These memcmp()s are going to fail if the ports differ. I guess that's okay, though, since we'll only tend to use this in cases where the ports are the same.

This makes me wonder whether grpc_resolved_address, which does include a port, is really the right type on which to be providing these functions. In principle, is seems like these operations should be done on only the addresses, not the ports. But I guess we don't have a better type on which to provide these methods, so I guess this is okay.

src/core/lib/surface/server.h, line 473 at r4 (raw file):

    // UpdateConnectionManager() is invoked by the config fetcher when a new
    // config is available. Implementations should update the connection manager
    // and start serving if not already serving. Ownership of \a args is

The last sentence can be removed from this comment, since args is no longer a parameter here.

src/proto/grpc/testing/xds/v3/listener.proto, line 60 at r4 (raw file):

message FilterChainMatch {
enum ConnectionSourceType {

Please fix indentation.

test/core/iomgr/sockaddr_utils_test.cc, line 1 at r4 (raw file):

/*

Please change all comments in this file to C++-style.

test/core/iomgr/sockaddr_utils_test.cc, line 39 at r4 (raw file):

namespace {

grpc_resolved_address make_addr4(const uint8_t* data, size_t data_len) {

Please rename to MakeAddr4().

test/core/iomgr/sockaddr_utils_test.cc, line 52 at r4 (raw file):

}

grpc_resolved_address make_addr6(const uint8_t* data, size_t data_len) {

MakeAddr6()

test/core/iomgr/sockaddr_utils_test.cc, line 65 at r4 (raw file):

}

void set_addr6_scope_id(grpc_resolved_address* addr, uint32_t scope_id) {

SetIPv6ScopeId()

test/core/iomgr/sockaddr_utils_test.cc, line 82 at r4 (raw file):

TEST(SockAddrUtilsTest, SockAddrIsV4Mapped) {
  grpc_resolved_address input4;

These can be declared as they are assigned.

test/core/iomgr/sockaddr_utils_test.cc, line 107 at r4 (raw file):

TEST(SockAddrUtilsTest, SockAddrToV4Mapped) {
  grpc_resolved_address input4;

These can be declared as they are assigned.

test/core/iomgr/sockaddr_utils_test.cc, line 112 at r4 (raw file):

  grpc_resolved_address expect6;

  gpr_log(GPR_INFO, "%s", "test_sockaddr_to_v4mapped");

This is no longer needed.

test/core/iomgr/sockaddr_utils_test.cc, line 132 at r4 (raw file):

TEST(SockAddrUtilsTest, SockAddrIsWildCard) {
  grpc_resolved_address wild4;

These can be declared as they are assigned.

test/core/iomgr/sockaddr_utils_test.cc, line 141 at r4 (raw file):

  int port;

  gpr_log(GPR_INFO, "%s", "test_sockaddr_is_wildcard");

No longer needed.

test/core/iomgr/sockaddr_utils_test.cc, line 178 at r4 (raw file):

}

void expect_sockaddr_str(const char* expected, grpc_resolved_address* addr,

This helper funciton is no longer needed. Instead, we can just inline EXPECT_EQ(grpc_sockaddr_to_string(addr, normalize), expected) in the callers.

test/core/iomgr/sockaddr_utils_test.cc, line 185 at r4 (raw file):

}

void expect_sockaddr_uri(const char* expected, grpc_resolved_address* addr) {

Same here.

test/core/iomgr/sockaddr_utils_test.cc, line 192 at r4 (raw file):

TEST(SockAddrUtilsTest, SockAddrToString) {
  grpc_resolved_address input4;

These can be declared as they are assigned.

test/core/iomgr/sockaddr_utils_test.cc, line 197 at r4 (raw file):

  grpc_sockaddr* phony_addr;

  gpr_log(GPR_INFO, "%s", "test_sockaddr_to_string");

No longer needed.

test/core/iomgr/sockaddr_utils_test.cc, line 240 at r4 (raw file):

TEST(SockAddrUtilsTest, SockAddrSetGetPort) {
  grpc_resolved_address input4;

These can be declared as they are assigned.

test/core/iomgr/sockaddr_utils_test.cc, line 245 at r4 (raw file):

  grpc_sockaddr* phony_addr;

  gpr_log(GPR_DEBUG, "test_sockaddr_set_get_port");

No longer needed.

test/core/iomgr/sockaddr_utils_test.cc, line 267 at r4 (raw file):

                              const std::string& subnet, uint32_t mask_bits,
                              bool success) {
  gpr_log(GPR_ERROR, "IP: %s Subnet: %s Mask: %u", ip_address.c_str(),

Instead of logging these parameters unconditionally (especially using GPR_ERROR), let's just have the EXPECT_EQ() statement at the end print them if the expectation fails (e.g., EXPECT_EQ(...) << "IP=" << ip_address << " Subnet=" << subnet << " Mask=" << mask_bits;).

test/cpp/end2end/xds_end2end_test.cc, line 8102 at r4 (raw file):

    socket_address->set_address(ipv6_only_ ? "::1" : "127.0.0.1");
    socket_address->set_port_value(backends_[0]->port());
    listener.add_filter_chains();

I think we can just remove this as well. We don't need an empty entry here.

test/cpp/end2end/xds_end2end_test.cc, line 8404 at r4 (raw file):

TEST_P(XdsServerFilterChainMatchTest,
       FilterChainsWithMoreSpecificPrefixRangesArePreferred) {

s/Prefix/DestinationPrefix/

test/cpp/end2end/xds_end2end_test.cc, line 8412 at r4 (raw file):

  socket_address->set_address(ipv6_only_ ? "::1" : "127.0.0.1");
  socket_address->set_port_value(backends_[0]->port());
  // Add filter chain with prefix range (lenth 16) but with server name

s/lenth/length/

test/cpp/end2end/xds_end2end_test.cc, line 8422 at r4 (raw file):

  prefix_range->mutable_prefix_len()->set_value(16);
  filter_chain->mutable_filter_chain_match()->add_server_names("server_name");
  // Add filter chain with two prefix ranges (length 8 and 24). Since 24 is the

How do we know that 24 will be matched and not 8? There's nothing in the entry for 8 that would disqualify the match later. Should we add a server_name field to it, like we do with the others that we don't want to match?

test/cpp/end2end/xds_end2end_test.cc, line 8470 at r4 (raw file):

  filter_chain->mutable_filter_chain_match()->set_source_type(
      FilterChainMatch::SAME_IP_OR_LOOPBACK);
  // Add filter chain with the external source type but bad source port.

Please add a comment explaining that we know the backend's port will not be the source port, because that port is in use by the backend. It took me a minute to figure out why this would work. :)

test/cpp/end2end/xds_end2end_test.cc, line 8499 at r4 (raw file):

  socket_address->set_address(ipv6_only_ ? "::1" : "127.0.0.1");
  socket_address->set_port_value(backends_[0]->port());
  // Add filter chain with source prefix range (lenth 16) but with a bad source

s/lenth/length/

test/cpp/end2end/xds_end2end_test.cc, line 8510 at r4 (raw file):

  filter_chain->mutable_filter_chain_match()->add_source_ports(
      backends_[0]->port());
  // Add filter chain with two source prefix ranges (length 8 and 24). Since 24

Similar comment to above: Should we add a bad source port to the one with length 8?

test/cpp/end2end/xds_end2end_test.cc, line 8556 at r4 (raw file):

  socket_address->set_address(ipv6_only_ ? "::1" : "127.0.0.1");
  socket_address->set_port_value(backends_[0]->port());
  // Add filter chain with source prefix range (lenth 16) but with a bad source

This comment looks like it was copied from above, but it doesn't seem to belong here.

test/cpp/end2end/xds_end2end_test.cc, line 8610 at r4 (raw file):

  EXPECT_THAT(response_state.error_message,
              ::testing::HasSubstr(
                  "Filter chains with duplicate matching rules detected"));

We'll want these error messages to be a bit more specific. They should at least indicate which filter chain entry (by index) caused the problem.

yashykt

Reviewable status: all files reviewed, 55 unresolved discussions (waiting on @markdroth and @yashykt)

src/core/ext/xds/xds_server_config_fetcher.cc, line 162 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

How about just using grpc_parse_uri() here?

We want to compare on the port separately and not as part of the grpc_resolved_address, so this was just convenient.

src/core/ext/xds/xds_server_config_fetcher.cc, line 169 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Are the casts actually needed here?

Yes, enum classes cannot be converted to int without an explicit cast. (I could use C style enums though but this seemed fine)

src/core/ext/xds/xds_server_config_fetcher.cc, line 203 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Need to first check that destination_uri.ok() is true.

Thanks!

src/core/ext/xds/xds_server_config_fetcher.cc, line 209 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

How about just using grpc_parse_uri() here?

I'm doing it this way mainly to avoid the port being added to the address

src/core/ext/xds/xds_server_config_fetcher.cc, line 320 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Why are we removing the credentials from args if they're not XdsCreds? Won't this break the ability to use xDS in the server without using XdsCreds? I think we still want to support that.

Let's make sure we have a test for this case.

This is not removing the credentials arg. It is removing the filter chain manager arg. (It's an artifact from a previous code iteration.) Removing.

Note that we do test an xDS enabled server without using Xds Creds. XdsEnabledServerTest does that.

src/core/ext/xds/xds_server_config_fetcher.cc, line 431 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

I think this can be written a little more clearly by inverting the second condition:

if (filter_chain_match_manager_ == nullptr ||
    listener.destination_ip_map !=
        filter_chain_match_manager_->destination_ip_map() ||
    listener.default_filter_chain !=
        filter_chain_match_manager_->default_filter_chain()) {

I get an error of the form

src/core/ext/xds/xds_server_config_fetcher.cc:427:43: error: invalid operands to binary expression ('absl::optional<FilterChain>' and 'const absl::optional<XdsApi::LdsUpdate::FilterChain>')
            listener.default_filter_chain !=

absl::optional does not like operator!= here.

src/core/lib/gprpp/atomic.h, line 84 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Why was this change necessary?

This was a very strange error, and I sequentially reverted different parts of my code to figure out what exactly was causing this.

So, the error itself is this -

                 from /var/local/git/grpc/src/core/lib/gprpp/atomic.h:24,
                 from /var/local/git/grpc/src/core/lib/gprpp/arena.h:38,
                 from /var/local/git/grpc/src/core/lib/channel/channel_stack.h:57,
                 from /var/local/git/grpc/src/core/ext/filters/client_channel/client_channel_channelz.h:27,
                 from /var/local/git/grpc/src/core/ext/filters/client_channel/client_channel.h:24,
                 from /var/local/git/grpc/src/core/ext/xds/xds_client.cc:35:
/usr/include/c++/4.9/bits/atomic_base.h: In member function 'bool grpc_core::Atomic<T>::IncrementIfNonzero(grpc_core::MemoryOrder) [with T = long int]':
/usr/include/c++/4.9/bits/atomic_base.h:538:70: error: failure memory model cannot be stronger than success memory model for '__atomic_compare_exchange'
  return __atomic_compare_exchange_n(&_M_i, &__i1, __i2, 1, __m1, __m2);```

and this seems to happen only with gcc-4.9. With some google search, turns out other folks have run into the same error (https://bugzilla.mozilla.org/show_bug.cgi?id=1029245 for example).

With all my code reversion, I narrowed it down some struct additions to xds_api.h. It looks like when the code structure gets complicated enough, gcc-4.9 fails to realize that we never use `IncrementIfNonzero` with a memory model other than `ACQUIRE`. Using a model other than ACQUIRE would probably be wrong in the first place for the manner that we are using it in, so I just removed the parameter. We can probably revisit this later if we need to do something special here. I suspect we will not need to.

src/core/lib/iomgr/sockaddr_utils.cc, line 383 at r2 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

These memcmp()s are going to fail if the ports differ. I guess that's okay, though, since we'll only tend to use this in cases where the ports are the same.

This makes me wonder whether grpc_resolved_address, which does include a port, is really the right type on which to be providing these functions. In principle, is seems like these operations should be done on only the addresses, not the ports. But I guess we don't have a better type on which to provide these methods, so I guess this is okay.

sin_addr does not include the port, so it should not fail. The same for sin6_addr.

But, I do agree that grpc_resolved_address is not ideal here. I was looking for a more concise type and we may want to add that in the future. For now, grpc_resolved_address and its utility functions are convenient :)

yashykt

Reviewable status: 8 of 17 files reviewed, 54 unresolved discussions (waiting on @markdroth and @yashykt)

src/core/ext/transport/chttp2/server/chttp2_server.cc, line 97 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Please call this argument connection_manager.

Done.

src/core/ext/transport/chttp2/server/chttp2_server.cc, line 721 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Need to return after this.

You are right. Thanks!

src/core/ext/transport/chttp2/server/chttp2_server.cc, line 721 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

I think this also needs to destroy args.

Done! Thanks!

src/core/ext/xds/xds_api.h, line 341 at r2 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

It might be helpful to have an overall comment here indicating the high-level view of this data structure. Something like this (if I understand correctly):

A multi-level map used to determine which filter chain to use for a given incoming connection.
Determining the right filter chain for a given connection checks the following properties, in order:
- destination port (never matched, so not present in map)
- destination IP range
- server name (never matched, so not present in map)
- transport protocol (allows only "raw_buffer" or unset, prefers the former)
- application protocol (never matched, so not present in map)
- connection source type
- source IP
- source port

I like it. Added the comment.

src/core/ext/xds/xds_api.h, line 354 at r2 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Suggest calling this ConnectionSourceTypesArray, to better match the enum type name used in FilterChainMatch above.

Done.

src/core/ext/xds/xds_api.h, line 383 at r2 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Do we still need this field if we're returning destination_ip_map? Doesn't that field contain the same thing, and won't the code that uses these fields only look at that one?

I only intended to use it for logging purposes. I thought that this struct has a limited lifetime (until the update is propagated to the config fetcher). I didn't realize that this is actually cached by the xds_client. Removed this, and added a logging function for the built nested map

src/core/ext/xds/xds_api.h, line 346 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Having a separate struct just to hold a shared_ptr seems a little cumbersome. It looks like the reason it's here is to allow comparisons for the overall struct to work correctly. But I wonder if we can avoid this by not using a shared_ptr to begin with. I'm thinking that we could instead do something like this:
struct SourcePortsMap {
  // Stores the data entries.
  std::vector<FilterChainData> storage;
  // Map from ports to entries in the storage field.
  std::map<uint16_t, FilterChainData*> map;
};
This both cleans up this struct and avoids the locking overhead of shared_ptr.

Yes, the reason for the separate struct is to allow for comparison. As discussed on GVC, accessing the shared_ptr is cheap and it makes the build algorithm really simple and straightforward compared to using a vector for storage (which would require a separate pass after building to remove the unused entries). Keeping the shared_ptr approach for now

src/core/ext/xds/xds_api.cc, line 2246 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Please change auto& to uint32_t.

Done.

src/core/ext/xds/xds_api.cc, line 2310 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Is the cast actually needed here?

Yes, enum classes are not implicitly convertible to int

src/core/ext/xds/xds_api.cc, line 2332 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

std::string&

Done.

src/core/ext/xds/xds_server_config_fetcher.cc, line 70 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Why do we need to save our own refs to the root and instance providers? Doesn't XdsCertificateProvider already internally hold refs to those?

No, it just takes a ref to their distributors.

src/core/ext/xds/xds_server_config_fetcher.cc, line 114 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

This should not be a reference, and const is unnecessary. Using a const reference here winds up triggering case 8 as described in https://abseil.io/tips/101. For more details, see https://abseil.io/tips/107.

Ah, you are right. Fixed it

src/core/ext/xds/xds_server_config_fetcher.cc, line 114 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Suggest calling this it to represent the fact that it's an iterator.

Done.

src/core/ext/xds/xds_server_config_fetcher.cc, line 119 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Same as above. Suggest just reusing the previous it variable here.

Done.

src/core/ext/xds/xds_server_config_fetcher.cc, line 209 at r4 (raw file):

Previously, yashykt (Yash Tibrewal) wrote…

I'm doing it this way mainly to avoid the port being added to the address

well, actually, in this case, we don't need the port so we should just use grpc_parse_uri. Thanks!

src/core/ext/xds/xds_server_config_fetcher.cc, line 307 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

default_filter_chain_ != nullptr

It's an absl::optional :)

src/core/ext/xds/xds_server_config_fetcher.cc, line 330 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

std::move()

Done.

src/core/lib/iomgr/sockaddr_utils.h, line 83 at r2 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Please document this function.

Done.

src/core/lib/iomgr/sockaddr_utils.h, line 86 at r2 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Same here.

Done.

src/core/lib/surface/server.h, line 473 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

The last sentence can be removed from this comment, since args is no longer a parameter here.

Done.

src/proto/grpc/testing/xds/v3/listener.proto, line 60 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Please fix indentation.

Done.

test/core/iomgr/sockaddr_utils_test.cc, line 1 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Please change all comments in this file to C++-style.

Done.

test/core/iomgr/sockaddr_utils_test.cc, line 39 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Please rename to MakeAddr4().

Done.

test/core/iomgr/sockaddr_utils_test.cc, line 52 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

MakeAddr6()

Done.

test/core/iomgr/sockaddr_utils_test.cc, line 65 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

SetIPv6ScopeId()

Done.

test/core/iomgr/sockaddr_utils_test.cc, line 82 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

These can be declared as they are assigned.

Done.

test/core/iomgr/sockaddr_utils_test.cc, line 107 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

These can be declared as they are assigned.

Done.

test/core/iomgr/sockaddr_utils_test.cc, line 112 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

This is no longer needed.

Done.

test/core/iomgr/sockaddr_utils_test.cc, line 132 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

These can be declared as they are assigned.

Done.

test/core/iomgr/sockaddr_utils_test.cc, line 141 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

No longer needed.

Done.

test/core/iomgr/sockaddr_utils_test.cc, line 178 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

This helper funciton is no longer needed. Instead, we can just inline EXPECT_EQ(grpc_sockaddr_to_string(addr, normalize), expected) in the callers.

You are right. Thanks! Done!

test/core/iomgr/sockaddr_utils_test.cc, line 185 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Same here.

Done.

test/core/iomgr/sockaddr_utils_test.cc, line 192 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

These can be declared as they are assigned.

Done.

test/core/iomgr/sockaddr_utils_test.cc, line 197 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

No longer needed.

Done.

test/core/iomgr/sockaddr_utils_test.cc, line 240 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

These can be declared as they are assigned.

Done.

test/core/iomgr/sockaddr_utils_test.cc, line 245 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

No longer needed.

Done.

test/core/iomgr/sockaddr_utils_test.cc, line 267 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Instead of logging these parameters unconditionally (especially using GPR_ERROR), let's just have the EXPECT_EQ() statement at the end print them if the expectation fails (e.g., EXPECT_EQ(...) << "IP=" << ip_address << " Subnet=" << subnet << " Mask=" << mask_bits;).

I forgot about this. Thanks! Done.

test/cpp/end2end/xds_end2end_test.cc, line 8102 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

I think we can just remove this as well. We don't need an empty entry here.

Done.

test/cpp/end2end/xds_end2end_test.cc, line 8404 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

s/Prefix/DestinationPrefix/

Done.

test/cpp/end2end/xds_end2end_test.cc, line 8412 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

s/lenth/length/

Done.

test/cpp/end2end/xds_end2end_test.cc, line 8422 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

How do we know that 24 will be matched and not 8? There's nothing in the entry for 8 that would disqualify the match later. Should we add a server_name field to it, like we do with the others that we don't want to match?

Both 8 and 24 are mentioned as prefix ranges for the same filter chain match. Adding server_name would apply it to both. That being said I understand the concern of matching the smaller prefix range instead. Added a prefix range of length 4 to one of the non-matching filters to verify.

test/cpp/end2end/xds_end2end_test.cc, line 8470 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Please add a comment explaining that we know the backend's port will not be the source port, because that port is in use by the backend. It took me a minute to figure out why this would work. :)

:) Done

test/cpp/end2end/xds_end2end_test.cc, line 8499 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

s/lenth/length/

Done.

test/cpp/end2end/xds_end2end_test.cc, line 8510 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Similar comment to above: Should we add a bad source port to the one with length 8?

Similarly added a prefix-range of length 4 to the non-matching one to make sure we are not preferring smaller prefix range lengths instead

test/cpp/end2end/xds_end2end_test.cc, line 8556 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

This comment looks like it was copied from above, but it doesn't seem to belong here.

:) Thanks!

yashykt

Reviewable status: 8 of 17 files reviewed, 54 unresolved discussions (waiting on @markdroth and @yashykt)

src/core/ext/xds/xds_server_config_fetcher.cc, line 157 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Need to check that source_uri.ok() is true first.

Done. Thanks!

src/core/ext/xds/xds_server_config_fetcher.cc, line 209 at r4 (raw file):

Previously, yashykt (Yash Tibrewal) wrote…

well, actually, in this case, we don't need the port so we should just use grpc_parse_uri. Thanks!

no, we still need the host to pass to the other function. So SplitHostPort remains convenient here

yashykt

Reviewable status: 8 of 17 files reviewed, 54 unresolved discussions (waiting on @markdroth)

test/cpp/end2end/xds_end2end_test.cc, line 8610 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

We'll want these error messages to be a bit more specific. They should at least indicate which filter chain entry (by index) caused the problem.

Done.

yashykt

Reviewable status: 8 of 17 files reviewed, 54 unresolved discussions (waiting on @markdroth)

src/core/ext/xds/xds_server_config_fetcher.cc, line 320 at r4 (raw file):

Previously, yashykt (Yash Tibrewal) wrote…

This is not removing the credentials arg. It is removing the filter chain manager arg. (It's an artifact from a previous code iteration.) Removing.

Note that we do test an xDS enabled server without using Xds Creds. XdsEnabledServerTest does that.

Done.

src/core/ext/xds/xds_server_config_fetcher.cc, line 431 at r4 (raw file):

Previously, yashykt (Yash Tibrewal) wrote…

I get an error of the form

src/core/ext/xds/xds_server_config_fetcher.cc:427:43: error: invalid operands to binary expression ('absl::optional<FilterChain>' and 'const absl::optional<XdsApi::LdsUpdate::FilterChain>')
            listener.default_filter_chain !=

absl::optional does not like operator!= here.

maybe it expects the operator!= to be implemented

src/core/lib/gprpp/atomic.h, line 84 at r4 (raw file):

Previously, yashykt (Yash Tibrewal) wrote…

This was a very strange error, and I sequentially reverted different parts of my code to figure out what exactly was causing this.

So, the error itself is this -

                 from /var/local/git/grpc/src/core/lib/gprpp/atomic.h:24,
                 from /var/local/git/grpc/src/core/lib/gprpp/arena.h:38,
                 from /var/local/git/grpc/src/core/lib/channel/channel_stack.h:57,
                 from /var/local/git/grpc/src/core/ext/filters/client_channel/client_channel_channelz.h:27,
                 from /var/local/git/grpc/src/core/ext/filters/client_channel/client_channel.h:24,
                 from /var/local/git/grpc/src/core/ext/xds/xds_client.cc:35:
/usr/include/c++/4.9/bits/atomic_base.h: In member function 'bool grpc_core::Atomic<T>::IncrementIfNonzero(grpc_core::MemoryOrder) [with T = long int]':
/usr/include/c++/4.9/bits/atomic_base.h:538:70: error: failure memory model cannot be stronger than success memory model for '__atomic_compare_exchange'
  return __atomic_compare_exchange_n(&_M_i, &__i1, __i2, 1, __m1, __m2);```

and this seems to happen only with gcc-4.9. With some google search, turns out other folks have run into the same error (https://bugzilla.mozilla.org/show_bug.cgi?id=1029245 for example).

With all my code reversion, I narrowed it down some struct additions to xds_api.h. It looks like when the code structure gets complicated enough, gcc-4.9 fails to realize that we never use `IncrementIfNonzero` with a memory model other than `ACQUIRE`. Using a model other than ACQUIRE would probably be wrong in the first place for the manner that we are using it in, so I just removed the parameter. We can probably revisit this later if we need to do something special here. I suspect we will not need to.


</blockquote></details>

(continuing previous comment.. bad formatting issues)
and this seems to happen only with gcc-4.9. With some google search, turns out other folks have run into the same error (https://bugzilla.mozilla.org/show_bug.cgi?id=1029245 for example).
 
With all my code reversion, I narrowed it down some struct additions to xds_api.h. It looks like when the code structure gets complicated enough, gcc-4.9 fails to realize that we never use `IncrementIfNonzero` with a memory model other than `ACQUIRE`. Using a model other than ACQUIRE would probably be wrong in the first place for the manner that we are using it in, so I just removed the parameter. We can probably revisit this later if we need to do something special here. I suspect we will not need to.


<!-- Sent from Reviewable.io -->

markdroth

This looks really good! My only remaining significant comments are about cleaning up the data structures in xds_api.h.

Please let me know if you have any questions. Thanks!

Reviewed 5 of 9 files at r5, 4 of 4 files at r6.
Reviewable status: all files reviewed, 13 unresolved discussions (waiting on @soheilhy and @yashykt)

src/core/ext/xds/xds_api.h, line 279 at r6 (raw file):

    };

    struct FilterChain {

This struct is no longer needed outside of the .cc file, so I suggest moving it there.

We'll need to leave ConnectionSourceType and FilterChainData here, since they're used below.

src/core/ext/xds/xds_api.h, line 309 at r6 (raw file):

        std::vector<std::string> application_protocols;

        bool operator==(const FilterChainMatch& other) const {

These two methods are probably not needed anymore once this struct moves to the .cc file.

src/core/ext/xds/xds_api.h, line 335 at r6 (raw file):

        std::string ToString() const;
      } filter_chain_data;

When you move this struct to the .cc file, consider making this field a shared_ptr<>. That way, you can just grab a ref when populating the new data structure, without having to make any copies.

src/core/ext/xds/xds_api.h, line 352 at r6 (raw file):

    };

    // A multi-level map used to determine which filter chain to use for a given

Please move this comment above FilterChainDataSharedPtr, since that type is part of this data structure too.

Actually, would it make sense to move all of these types into a nested struct, so that it's clear that they're all related?

src/core/ext/xds/xds_api.h, line 359 at r6 (raw file):

    // - server name (never matched, so not present in map)
    // - transport protocol (allows only "raw_buffer" or unset, prefers the
    // former)

Please indent this word two spaces, to line up with the text above.

src/core/ext/xds/xds_api.h, line 409 at r6 (raw file):

    std::string address;
    DestinationIpMap destination_ip_map;
    absl::optional<FilterChain> default_filter_chain;

I think this can be FilterChainData, since it doesn't need any of the fields from FilterChainMatch.

src/core/ext/xds/xds_api.cc, line 682 at r6 (raw file):

              absl::StrCat("filter_chain_match={",
                           absl::StrJoin(filter_chain_match_content, ", "),
                           "}, ", source_port_pair.second.data->ToString()));

I think we don't need the comma after the close-brace here, since we'll add the commas in absl::StrJoin() below.

src/core/ext/xds/xds_api.cc, line 2276 at r6 (raw file):

grpc_error* AddFilterChainDataForSourcePort(
    const XdsApi::LdsUpdate::FilterChain::FilterChainMatch& filter_chain_match,

How about just passing in the FilterChain struct, combining these first two parameters?

Same for most of these helper functions.

src/core/ext/xds/xds_api.cc, line 2470 at r6 (raw file):

grpc_error* BuildFilterChainMatchStructure(
    const std::vector<XdsApi::LdsUpdate::FilterChain> filter_chains,

This should be a reference to avoid an unnecessary copy.

src/core/ext/xds/xds_server_config_fetcher.cc, line 70 at r4 (raw file):

Previously, yashykt (Yash Tibrewal) wrote…

No, it just takes a ref to their distributors.

Okay. Please add a comment about that.

src/core/ext/xds/xds_server_config_fetcher.cc, line 431 at r4 (raw file):

Previously, yashykt (Yash Tibrewal) wrote…

maybe it expects the operator!= to be implemented

Ah, right -- we're not implementing != for these structs. Okay.

src/core/lib/gprpp/atomic.h, line 84 at r4 (raw file):

Previously, yashykt (Yash Tibrewal) wrote…

(continuing previous comment.. bad formatting issues)
and this seems to happen only with gcc-4.9. With some google search, turns out other folks have run into the same error (https://bugzilla.mozilla.org/show_bug.cgi?id=1029245 for example).

With all my code reversion, I narrowed it down some struct additions to xds_api.h. It looks like when the code structure gets complicated enough, gcc-4.9 fails to realize that we never use IncrementIfNonzero with a memory model other than ACQUIRE. Using a model other than ACQUIRE would probably be wrong in the first place for the manner that we are using it in, so I just removed the parameter. We can probably revisit this later if we need to do something special here. I suspect we will not need to.

Okay. This looks fine to me, but just to be safe, let's ask @soheilhy to make sure there are no hidden gotchas here.

test/core/iomgr/sockaddr_utils_test.cc, line 18 at r6 (raw file):

// With the addition of a libuv endpoint, sockaddr.h now includes uv.h when
//   using that endpoint. Because of various transitive includes in uv.h,

The rest of this comment doesn't need to be indented anymore.

test/core/iomgr/sockaddr_utils_test.cc, line 166 at r6 (raw file):

  grpc_resolved_address input4 = MakeAddr4(kIPv4, sizeof(kIPv4));
  EXPECT_EQ(grpc_sockaddr_to_string(&input4, 0), "192.0.2.1:12345");

Please use true and false instead of 0 and 1 for the normalize argument.

Same throughout.

soheilhy

Reviewable status: all files reviewed, 13 unresolved discussions (waiting on @markdroth and @yashykt)

src/core/lib/gprpp/atomic.h, line 84 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Okay. This looks fine to me, but just to be safe, let's ask @soheilhy to make sure there are no hidden gotchas here.

Thanks for checking. I have no objection.

That error should only show up if load_order is an invalid ordering with respect to failures. Using ACQUIRE is safe but it's unnecessary for some cases that have a fast path on cmpxchg failure. That's why there was an option to set this. If there are currently no user of this API, it's probably not as useful as we thought it would be.

yashykt

Reviewable status: 13 of 17 files reviewed, 13 unresolved discussions (waiting on @markdroth)

src/core/ext/xds/xds_api.h, line 279 at r6 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

This struct is no longer needed outside of the .cc file, so I suggest moving it there.

We'll need to leave ConnectionSourceType and FilterChainData here, since they're used below.

Right, moved it.

src/core/ext/xds/xds_api.h, line 309 at r6 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

These two methods are probably not needed anymore once this struct moves to the .cc file.

true for operator== but the ToString() method is still being used

src/core/ext/xds/xds_api.h, line 335 at r6 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

When you move this struct to the .cc file, consider making this field a shared_ptr<>. That way, you can just grab a ref when populating the new data structure, without having to make any copies.

Agreed, done.

src/core/ext/xds/xds_api.h, line 352 at r6 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Please move this comment above FilterChainDataSharedPtr, since that type is part of this data structure too.

Actually, would it make sense to move all of these types into a nested struct, so that it's clear that they're all related?

I like that suggestion. Adopted it. Moved them under FilterChainMap

src/core/ext/xds/xds_api.h, line 359 at r6 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Please indent this word two spaces, to line up with the text above.

Done.

src/core/ext/xds/xds_api.h, line 409 at r6 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

I think this can be FilterChainData, since it doesn't need any of the fields from FilterChainMatch.

You are right. Done!

src/core/ext/xds/xds_api.cc, line 682 at r6 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

I think we don't need the comma after the close-brace here, since we'll add the commas in absl::StrJoin() below.

Note that we are adding source_port_pair.second.data->ToString()after the comma. (FilterChainData, but not mentioning FilterChainData to avoid user confusion since the proto does not have that)

src/core/ext/xds/xds_api.cc, line 2276 at r6 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

How about just passing in the FilterChain struct, combining these first two parameters?

Same for most of these helper functions.

Done.

src/core/ext/xds/xds_api.cc, line 2470 at r6 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

This should be a reference to avoid an unnecessary copy.

Good catch!

src/core/ext/xds/xds_server_config_fetcher.cc, line 70 at r4 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Okay. Please add a comment about that.

Done.

src/core/lib/gprpp/atomic.h, line 84 at r4 (raw file):

Previously, soheilhy (Soheil Hassas Yeganeh) wrote…

Thanks for checking. I have no objection.

That error should only show up if load_order is an invalid ordering with respect to failures. Using ACQUIRE is safe but it's unnecessary for some cases that have a fast path on cmpxchg failure. That's why there was an option to set this. If there are currently no user of this API, it's probably not as useful as we thought it would be.

Ack

test/core/iomgr/sockaddr_utils_test.cc, line 18 at r6 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

The rest of this comment doesn't need to be indented anymore.

I usually just leave it to clang-format to figure out the right thing to do :)

test/core/iomgr/sockaddr_utils_test.cc, line 166 at r6 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Please use true and false instead of 0 and 1 for the normalize argument.

Same throughout.

Done.

markdroth

This is pretty close now! Remaining comments are mostly minor.

Please let me know if you have any questions. Thanks!

Reviewed 3 of 4 files at r7, 1 of 1 files at r8.
Reviewable status: all files reviewed, 12 unresolved discussions (waiting on @yashykt)

src/core/ext/xds/xds_api.h, line 286 at r8 (raw file):

    std::string address;

    enum class ConnectionSourceType { kAny = 0, kSameIpOrLoopback, kExternal };

This can be moved into FilterChainMap.

src/core/ext/xds/xds_api.cc, line 682 at r6 (raw file):

Previously, yashykt (Yash Tibrewal) wrote…

Note that we are adding source_port_pair.second.data->ToString()after the comma. (FilterChainData, but not mentioning FilterChainData to avoid user confusion since the proto does not have that)

Oh, sorry, I misread this.

If we stick with this flattened representation (see my comment elsewhere), then I think we should format it as "{filter_chain_match={...}, filter_chain={...}}". That will be more readable when logged, especially when there are multiple entries combined together.

src/core/ext/xds/xds_api.cc, line 580 at r8 (raw file):

           destination_ip_pair.second.source_types_array[source_type]) {
        for (const auto& source_port_pair : source_ip_pair.second.ports_map) {
          absl::InlinedVector<std::string, 5> filter_chain_match_content;

It looks like we're flattening this map out for logging. Why not just log it the way it actually is represented in this data structure?

src/core/ext/xds/xds_api.cc, line 628 at r8 (raw file):

    contents.push_back(absl::StrCat("address=", address));
    contents.push_back(
        absl::StrCat("filter_chains=", filter_chain_map.ToString()));

Suggest changing "filter_chains=" to "filter_chain_map=".

src/core/ext/xds/xds_api.cc, line 2027 at r8 (raw file):

    struct CidrRange {
      std::string address_prefix;

Would it make sense to just use grpc_resolved_address here, so that we don't have to convert to that form later?

src/core/ext/xds/xds_api.cc, line 2030 at r8 (raw file):

      uint32_t prefix_len;

      bool operator==(const CidrRange& other) const {

Is this still needed? I don't see it being used anywhere.

src/core/ext/xds/xds_api.cc, line 2047 at r8 (raw file):

    std::vector<std::string> application_protocols;

    std::string ToString() const;

I don't see this actually being called anywhere, so I don't think we need it.

Same for CidrRange::ToString() above.

src/core/ext/xds/xds_api.cc, line 2301 at r8 (raw file):

    XdsApi::LdsUpdate::FilterChainMap::SourcePortsMap* ports_map,
    uint32_t port) {
  std::pair<XdsApi::LdsUpdate::FilterChainMap::SourcePortsMap::iterator, bool>

This is a good case in which to use auto.

src/core/ext/xds/xds_api.cc, line 2334 at r8 (raw file):

    XdsApi::LdsUpdate::FilterChainMap::SourceIpMap* source_ip_map) {
  if (filter_chain.filter_chain_match.source_prefix_ranges.empty()) {
    std::pair<XdsApi::LdsUpdate::FilterChainMap::SourceIpMap::iterator, bool>

auto

src/core/ext/xds/xds_api.cc, line 2359 at r8 (raw file):

              ? uint32_t(32)
              : uint32_t(128));
      std::pair<XdsApi::LdsUpdate::FilterChainMap::SourceIpMap::iterator, bool>

auto

src/core/ext/xds/xds_api.cc, line 2436 at r8 (raw file):

    XdsApi::LdsUpdate::FilterChainMap::DestinationIpMap* destination_ip_map) {
  if (filter_chain.filter_chain_match.prefix_ranges.empty()) {
    std::pair<XdsApi::LdsUpdate::FilterChainMap::DestinationIpMap::iterator,

auto

src/core/ext/xds/xds_api.cc, line 2461 at r8 (raw file):

              ? uint32_t(32)
              : uint32_t(128));
      std::pair<XdsApi::LdsUpdate::FilterChainMap::DestinationIpMap::iterator,

auto

yashykt

Reviewed 2 of 16 files at r1, 1 of 9 files at r5, 3 of 4 files at r7, 1 of 1 files at r8.
Reviewable status: 14 of 17 files reviewed, 12 unresolved discussions (waiting on @markdroth and @yashykt)

src/core/ext/xds/xds_api.h, line 286 at r8 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

This can be moved into FilterChainMap.

Done.

src/core/ext/xds/xds_api.cc, line 682 at r6 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Oh, sorry, I misread this.

If we stick with this flattened representation (see my comment elsewhere), then I think we should format it as "{filter_chain_match={...}, filter_chain={...}}". That will be more readable when logged, especially when there are multiple entries combined together.

Done.

src/core/ext/xds/xds_api.cc, line 580 at r8 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

It looks like we're flattening this map out for logging. Why not just log it the way it actually is represented in this data structure?

I'm not sure if that would be readable/understandable

src/core/ext/xds/xds_api.cc, line 628 at r8 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Suggest changing "filter_chains=" to "filter_chain_map=".

Done.

src/core/ext/xds/xds_api.cc, line 2027 at r8 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Would it make sense to just use grpc_resolved_address here, so that we don't have to convert to that form later?

We can, but then we'll need to convert it back using grpc_sockaddr_to_string in that one place for logging (when we find it to be a duplicate).

src/core/ext/xds/xds_api.cc, line 2030 at r8 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Is this still needed? I don't see it being used anywhere.

Right, we don't. Removing.

src/core/ext/xds/xds_api.cc, line 2047 at r8 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

I don't see this actually being called anywhere, so I don't think we need it.

Same for CidrRange::ToString() above.

We do in one place for logging purposes (when we find it to be a duplicate..)

src/core/ext/xds/xds_api.cc, line 2301 at r8 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

This is a good case in which to use auto.

I don't know. The declaration for 'emplace' is not immediately obvious atleast to me, and I always have to look it up. Changed it anyway.

src/core/ext/xds/xds_api.cc, line 2334 at r8 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

auto

Done.

src/core/ext/xds/xds_api.cc, line 2359 at r8 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

auto

Done.

src/core/ext/xds/xds_api.cc, line 2436 at r8 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

auto

Done.

src/core/ext/xds/xds_api.cc, line 2461 at r8 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

auto

Done.

markdroth

Getting very close! The only significant remaining issues are about the representation of the IP prefixes and avoiding duplicate logging code.

Please let me know if you have any questions. Thanks!

Reviewed 3 of 3 files at r9.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @yashykt)

src/core/ext/xds/xds_api.h, line 332 at r9 (raw file):

        }
      };
      using SourceIpMap = std::map<std::string, SourceIp>;

I just noticed this: Why is this a map instead of a vector? It looks like our code needs to iterate over every entry anyway, so there's no reason it needs to be a map, and no real benefit to storing the human-readable string as a key.

src/core/ext/xds/xds_api.h, line 359 at r9 (raw file):

      };
      // We always fail match on destination ports map
      using DestinationIpMap = std::map<std::string, DestinationIp>;

Same question here.

src/core/ext/xds/xds_api.cc, line 682 at r6 (raw file):

Previously, yashykt (Yash Tibrewal) wrote…

Done.

Please also add braces around the entire thing. Otherwise, it's hard to tell where one ends and the next begins when we log the entire map.

src/core/ext/xds/xds_api.cc, line 2027 at r8 (raw file):

Previously, yashykt (Yash Tibrewal) wrote…

We can, but then we'll need to convert it back using grpc_sockaddr_to_string in that one place for logging (when we find it to be a duplicate).

I'm fine with converting it back for logging purposes. I think that we'll need to do the same thing in FilterChainMap::ToString() once you change the maps to vectors for the IP prefixes.

src/core/ext/xds/xds_api.cc, line 2047 at r8 (raw file):

Previously, yashykt (Yash Tibrewal) wrote…

We do in one place for logging purposes (when we find it to be a duplicate..)

Hmm. Can we do something to eliminate the duplication between this and FilterChainMap::ToString()? They're both logging basically the same data, so it would be nice to not have to have essentially the same code twice.

src/core/ext/xds/xds_api.cc, line 2436 at r8 (raw file):

Previously, yashykt (Yash Tibrewal) wrote…

Done.

I don't see a change here.

src/core/ext/xds/xds_api.cc, line 2461 at r8 (raw file):

Previously, yashykt (Yash Tibrewal) wrote…

Done.

I don't see a change here.

yashykt

Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @markdroth and @yashykt)

src/core/ext/xds/xds_api.h, line 332 at r9 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

I just noticed this: Why is this a map instead of a vector? It looks like our code needs to iterate over every entry anyway, so there's no reason it needs to be a map, and no real benefit to storing the human-readable string as a key.

Well, the benefit is mainly at build time where we can detect duplicate matches thanks to the match on the string address. That being said, I can also change this to a map which goes from SourceIP to SourcePortsMap

src/core/ext/xds/xds_api.h, line 359 at r9 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Same question here.

I can change it to a map which goes from DestinationIp to another middle type that holds the bool and source_types_array

src/core/ext/xds/xds_api.cc, line 2047 at r8 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Hmm. Can we do something to eliminate the duplication between this and FilterChainMap::ToString()? They're both logging basically the same data, so it would be nice to not have to have essentially the same code twice.

It's the same data but from a different structure. I can construct a FilterChainMatch from each FilterChainMap entry but that seems unnecessary

yashykt

Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @markdroth and @yashykt)

src/core/ext/xds/xds_api.h, line 332 at r9 (raw file):

Previously, yashykt (Yash Tibrewal) wrote…

Well, the benefit is mainly at build time where we can detect duplicate matches thanks to the match on the string address. That being said, I can also change this to a map which goes from SourceIP to SourcePortsMap

Well, actually I remembered why I'm storing a std::string as the key instead of SouceIp. It was handle the case of no prefix range being mentioned for a filter chain

markdroth

Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @yashykt)

src/core/ext/xds/xds_api.h, line 332 at r9 (raw file):

Previously, yashykt (Yash Tibrewal) wrote…

Well, actually I remembered why I'm storing a std::string as the key instead of SouceIp. It was handle the case of no prefix range being mentioned for a filter chain

I don't think it makes sense to have the extra overhead of a map and all of the human-readable keys beyond validation time if that's the only time that they're needed. If the map is really useful at validation time, then how about creating the map as a local variable during validation and then converting it into a smaller form once we're done validating?

Once we're done with validation, I think it can just be a vector of SourceIp structs. If we're not doing any random access into the map, it doesn't need to be a map in the first place.

I think we can handle the case of no prefix range being mentioned in a filter change by using absl::optional<>. For example, you could move the address and prefix_len fields into their own struct, and then both SourceIp and DestinationIp could have an absl::optional<> of that struct.

src/core/ext/xds/xds_api.h, line 359 at r9 (raw file):

Previously, yashykt (Yash Tibrewal) wrote…

I can change it to a map which goes from DestinationIp to another middle type that holds the bool and source_types_array

Same here -- I think it can just be a vector of DestinationIp structs once validation is done.

src/core/ext/xds/xds_api.cc, line 2047 at r8 (raw file):

Previously, yashykt (Yash Tibrewal) wrote…

It's the same data but from a different structure. I can construct a FilterChainMatch from each FilterChainMap entry but that seems unnecessary

That may actually be less code than the amount that we're currently duplicating. Maybe try it and see how it compares?

yashykt

Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @markdroth and @yashykt)

src/core/ext/xds/xds_api.h, line 332 at r9 (raw file):

If the map is really useful at validation time, then how about creating the map as a local variable during validation and then converting it into a smaller form once we're done validating?

The local variable needs to be at the scope of the respective map. So, there are two ways I can do that -

Create another temporary field (maybe a unique_ptr) which stores the map at build time, and when we are done building the map, we loop through the map and convert each map to a std::vector
Create a whole new type like TemporaryFilterChainMap which holds this temporary map and then converts it to FilterChainMap later when we are done validating

yashykt

Reviewable status: 13 of 17 files reviewed, 7 unresolved discussions (waiting on @markdroth)

src/core/ext/xds/xds_api.h, line 332 at r9 (raw file):

Previously, yashykt (Yash Tibrewal) wrote…

If the map is really useful at validation time, then how about creating the map as a local variable during validation and then converting it into a smaller form once we're done validating?

The local variable needs to be at the scope of the respective map. So, there are two ways I can do that -

Create another temporary field (maybe a unique_ptr) which stores the map at build time, and when we are done building the map, we loop through the map and convert each map to a std::vector

Create a whole new type like TemporaryFilterChainMap which holds this temporary map and then converts it to FilterChainMap later when we are done validating

Went with the second approach and created an InternalFilterChainMap which is used for validating the filter chain entries and finally building a stripped down FilterChainMap

src/core/ext/xds/xds_api.h, line 359 at r9 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Same here -- I think it can just be a vector of DestinationIp structs once validation is done.

Done.

src/core/ext/xds/xds_api.cc, line 682 at r6 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Please also add braces around the entire thing. Otherwise, it's hard to tell where one ends and the next begins when we log the entire map.

Done.

src/core/ext/xds/xds_api.cc, line 2027 at r8 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

I'm fine with converting it back for logging purposes. I think that we'll need to do the same thing in FilterChainMap::ToString() once you change the maps to vectors for the IP prefixes.

Done.

src/core/ext/xds/xds_api.cc, line 2047 at r8 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

That may actually be less code than the amount that we're currently duplicating. Maybe try it and see how it compares?

It does save 8 lines of code :) Adopted it

src/core/ext/xds/xds_api.cc, line 2436 at r8 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

I don't see a change here.

Looks like the search and replace missed it due to spacing. This time for sure

src/core/ext/xds/xds_api.cc, line 2461 at r8 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

I don't see a change here.

This time for sure.

markdroth

This looks great! Just a few minor nits remaining.

Please let me know if you have any questions. Thanks!

Reviewed 4 of 4 files at r10.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @yashykt)

src/core/ext/xds/xds_api.h, line 239 at r10 (raw file):

  };

  struct CidrRange {

This should be moved inside of the FilterChainMap struct.

src/core/ext/xds/xds_api.cc, line 598 at r10 (raw file):

//
// FilterChain::FilterChainMatch

Please move this comment above the FilterChain definition above, and just have it say FilterChain. We can consider FilterChainMatch to be part of FilterChain for the purposes of this comment.

src/core/ext/xds/xds_api.cc, line 2094 at r10 (raw file):

  std::string address_prefix = UpbStringToStdString(
      envoy_config_core_v3_CidrRange_address_prefix(cidr_range_proto));
  memset(&cidr_range->address, 0, sizeof(grpc_resolved_address));

This looks almost identical to what grpc_string_to_sockaddr() does. The only difference is that grpc_string_to_sockaddr() asserts if the address string can't be parsed, which is actually pretty horrible behavior -- I suggest changing it to return an error in that case, so that we can use it safely here.

If this winds up being non-trivial (e.g., changing grpc_string_to_sockaddr() requires changing a bunch of callers), then it's fine to just leave a TODO for now.

yashykt

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @markdroth and @yashykt)

src/core/ext/xds/xds_api.h, line 239 at r10 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

This should be moved inside of the FilterChainMap struct.

I actually moved it out earlier since CidrRange is not nested in FilterChain in the proto structure either

markdroth

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @yashykt)

src/core/ext/xds/xds_api.h, line 239 at r10 (raw file):

Previously, yashykt (Yash Tibrewal) wrote…

I actually moved it out earlier since CidrRange is not nested in FilterChain in the proto structure either

Sure, but that doesn't mean it needs to be separate here. We can move it out later if we wind up using it somewhere else, but for now let's keep it localized to the only place we're actually using it.

yashykt

Reviewable status: 13 of 17 files reviewed, 3 unresolved discussions (waiting on @markdroth)

src/core/ext/xds/xds_api.h, line 239 at r10 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Sure, but that doesn't mean it needs to be separate here. We can move it out later if we wind up using it somewhere else, but for now let's keep it localized to the only place we're actually using it.

Done.

src/core/ext/xds/xds_api.cc, line 598 at r10 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Please move this comment above the FilterChain definition above, and just have it say FilterChain. We can consider FilterChainMatch to be part of FilterChain for the purposes of this comment.

Done.

src/core/ext/xds/xds_api.cc, line 2094 at r10 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

This looks almost identical to what grpc_string_to_sockaddr() does. The only difference is that grpc_string_to_sockaddr() asserts if the address string can't be parsed, which is actually pretty horrible behavior -- I suggest changing it to return an error in that case, so that we can use it safely here.

If this winds up being non-trivial (e.g., changing grpc_string_to_sockaddr() requires changing a bunch of callers), then it's fine to just leave a TODO for now.

Agreed. Added a new variant grpc_string_to_sockaddr_new for now

markdroth

Just one minor nit remaining!

Reviewed 4 of 4 files at r11.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @yashykt)

src/core/ext/xds/xds_api.cc, line 2094 at r10 (raw file):

Previously, yashykt (Yash Tibrewal) wrote…

Agreed. Added a new variant grpc_string_to_sockaddr_new for now

Instead of a new variant, how about just adding an optional parameter to control whether it fails on error? As long as the default is still the current behavior, then it shouldn't break anything.

yashykt

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @markdroth)

src/core/ext/xds/xds_api.cc, line 2094 at r10 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Instead of a new variant, how about just adding an optional parameter to control whether it fails on error? As long as the default is still the current behavior, then it shouldn't break anything.

I intend to send a follow-up PR to remove and replace. Didn't want to do it here since there are more cpython usages of it than Core usages. So, let's keep it as it is here.

markdroth

This looks fantastic!

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @yashykt)

src/core/ext/xds/xds_api.cc, line 2094 at r10 (raw file):

Previously, yashykt (Yash Tibrewal) wrote…

I intend to send a follow-up PR to remove and replace. Didn't want to do it here since there are more cpython usages of it than Core usages. So, let's keep it as it is here.

Okay, as long as you make sure to follow up on it. Thanks!

yashykt · 2021-03-25T21:09:42Z

Thanks for reviewing!
I'll follow-up with removing and replacing grpc_string_to_sockaddr

* Implement FilterChainMatch logic * Add tests for transport protocol too * Tests for duplicate NACKing * Introduce ConnectionManager as an interface for config fetchers * Do not parameterize IncrementIfNonZero * Some formatting * Reviewer comments * Add filter chain match information for duplicate match error * Reviewer comments * Some cleanup * Reviewer comments * Reviewer comments * Reviewer comments * Clang-tidy

* Implement FilterChainMatch algorithm (#25757) (#25815) * Implement FilterChainMatch logic * Add tests for transport protocol too * Tests for duplicate NACKing * Introduce ConnectionManager as an interface for config fetchers * Do not parameterize IncrementIfNonZero * Some formatting * Reviewer comments * Add filter chain match information for duplicate match error * Reviewer comments * Some cleanup * Reviewer comments * Reviewer comments * Reviewer comments * Clang-tidy * Bump to 1.37.0-pre1 (#25839) * Bump to v1.37.0-pre1 * Regenerate projects * Use Realtime instead of Monotonic time for CSDS (#25864) * Add ruby 3.0 support for mac binary packages (#25869) * Bump to 1.37.0 for Final Release (#25891) * Bump version to 1.37.0 * Regenerate projects * PHP: Fix windows build (#25904) * [Backport] xdsinterop: extend the ports to use (#25916) This is to add more ports for forwarding-rule. It's in theory not necessary, because forwarding-rule doesn't need to use the same port as the services. This is a limitation of the test framework, and can be fixed in the future. * try/catch exceptions of both php7 and php8 (#25918) (#25927) * Fix use-after-unref bug in fault_injection_filter (#25903) (#25935) Co-authored-by: Yash Tibrewal <yashkt@google.com> * Fix fault injection filter to still run the original trailing metadata closure when no error (#25933) * also build python3.6 aarch64 manylinux2014 wheel (#25944) * Enable channelz for xds_interop_client and xds_interop_server (#25939) (#25968) * Enable channelz for xds_interop_client and xds_interop_server * Regenerate projects * Fix #25897 to avoid crashes when certificates are not yet updated (#25899) (#25965) * xds: move path_matching and header_matching to all; add fault_injection to ruby (#26030) * Backport several additions / fixes to PHP / Ruby xDS Interop tests (#26037) * PHP: allow xDS interop client to start RPCs asynchronously (#25696) * PHP: allow xDS interop client to start RPCs asynchronously * Address review comments * Remove adhoc test config * PHP: enable fault_injection xds interop test case (#25943) * PHP: enable fault_injection xds interop test case * sanity check yaph_code.sh * some mysterious SIGTERM is being observed * Remove adhoc test cfg * Ruby: Fix xds fault_injection test (#26006) * Fix header_matching for PHP and Ruby (#26017) * [Backport][1.37.x] Add kokoro job for xds psm security tests (#26035) * Add kokoro job for xds psm security tests (#26033) * add kokoro job for xds psm security * Fix scripts * Cleanup * Add copyright * Use existing repo for test driver (#26041) * Use existing repo for test driver * Reviewer comments * Don't check local certs for xds k8s kokoro job (#26046) * xds k8s kokoro - Configure auth for docker (#26050) * xds k8s kokoro - Configure auth for docker * Add -q * xds k8s kokoro job - Add git commit as tag for images (#26051) * [Backport][1.37.x] test_csds (#26047) * Add CSDS xDS interop test (#26007) * Add CSDS xDS interop test * Add CSDS test to the test suite * Fix a typo * Address comments * Improve the logging of each attempt * Improve Python readability * Fix a typo in CSDS test (#26021) * Update the Python dependency for xDS interop test (#26024) `xds-protos` includes all the generated Python files for Envoy protos, which is required by Envoy. * Upgrade setuptools and ProtoBuf Python in prep_xds.sh (#26029) * Upgrade * Trick Kokoro to run xDS test * Restore grpc_python_bazel_test.cfg * Update protobuf * Add extra layer of protection * [C++] Add admin and reflection to xds interop binaries (#25964) * Add admin and reflection to xds interop binaries * Prepare for #25978 * Remove _xds rules * Import json_format * [Backport][1.37.x] Backport xds-k8s driver changes (#26067) * xds-k8s: Update Private CA GKE workload certificates config (#25875) * xds-k8s: Update GKE workload certificates: fix annotation (#25882) * xds-k8s: Use latest TD bootstrap supporting new secrets dir (#25925) * Naming fix (secrets manager -> secret manager) (#25990) Co-authored-by: Seth Vargo <seth@sethvargo.com> * temporarily change ::createInsecure() back to return NULL (#26054) * buildscripts: xds-k8s pin pip to 21.0.1 (#26088) pip 21.1 released on Apr 24 introduced a regression for python 3.6.1. The regression was identified on Apr 24, the fix merged on Apr 25. The fix is expected to be delivered in the 21.1.1 patch. There's no clear date, when 21.1.1 will be released. Until then, pin is temporarily pinned to the previous release, 21.0.1. * Bump to 1.37.1 (#26040) * Bump to 1.37.1 * Regenerate projects * [Backport][1.37.x] xds-k8s buildscripts sync (#26103) * Python PSM Security Interop Client+Server (#25991) * Add channelz to security client * Make client changes. Some config needs to be reverted * Add missing security field to channelz Socket * Add test * Fix test * Remove whitespaces for windows * Remove local_certificate check from PSM security tests * Byte pack Python channelz IPs * Move address packing to Core * WIP * Unbork security tests * Python interop server * Turn up server logging * Clean up PR * Clean up some more * Yapf * Fix up docker images * Swap fillllllles! * Add more robust boolean arg parsing * Use bool_arg parsing in server as well * Add copyright Co-authored-by: Yash Tibrewal <yashkt@google.com> * ADD Python xDS security interop test CI scripts (#26073) * xds-k8s kokoro buildscripts: exclude from tests suites (#26098) * xds-k8s buildscript should use the latest version of the driver (#26100) Co-authored-by: Richard Belleville <rbellevi@google.com> Co-authored-by: Yash Tibrewal <yashkt@google.com> Co-authored-by: Richard Belleville <rbellevi@google.com> Co-authored-by: Lidi Zheng <lidiz@google.com> Co-authored-by: apolcyn <apolcyn@google.com> Co-authored-by: Stanley Cheung <stanleycheung@google.com> Co-authored-by: Menghan Li <menghanl@google.com> Co-authored-by: Hannah Shi <hannahshisfb@gmail.com> Co-authored-by: Jan Tattermusch <jtattermusch@users.noreply.github.com> Co-authored-by: Doug Fawley <dfawley@google.com> Co-authored-by: Sergii Tkachenko <sergiitk@google.com> Co-authored-by: Seth Vargo <seth@sethvargo.com>

* Implement FilterChainMatch algorithm (grpc#25757) (grpc#25815) * Implement FilterChainMatch logic * Add tests for transport protocol too * Tests for duplicate NACKing * Introduce ConnectionManager as an interface for config fetchers * Do not parameterize IncrementIfNonZero * Some formatting * Reviewer comments * Add filter chain match information for duplicate match error * Reviewer comments * Some cleanup * Reviewer comments * Reviewer comments * Reviewer comments * Clang-tidy * Bump to 1.37.0-pre1 (grpc#25839) * Bump to v1.37.0-pre1 * Regenerate projects * Use Realtime instead of Monotonic time for CSDS (grpc#25864) * Add ruby 3.0 support for mac binary packages (grpc#25869) * Bump to 1.37.0 for Final Release (grpc#25891) * Bump version to 1.37.0 * Regenerate projects * PHP: Fix windows build (grpc#25904) * [Backport] xdsinterop: extend the ports to use (grpc#25916) This is to add more ports for forwarding-rule. It's in theory not necessary, because forwarding-rule doesn't need to use the same port as the services. This is a limitation of the test framework, and can be fixed in the future. * try/catch exceptions of both php7 and php8 (grpc#25918) (grpc#25927) * Fix use-after-unref bug in fault_injection_filter (grpc#25903) (grpc#25935) Co-authored-by: Yash Tibrewal <yashkt@google.com> * Fix fault injection filter to still run the original trailing metadata closure when no error (grpc#25933) * also build python3.6 aarch64 manylinux2014 wheel (grpc#25944) * Enable channelz for xds_interop_client and xds_interop_server (grpc#25939) (grpc#25968) * Enable channelz for xds_interop_client and xds_interop_server * Regenerate projects * Fix grpc#25897 to avoid crashes when certificates are not yet updated (grpc#25899) (grpc#25965) * xds: move path_matching and header_matching to all; add fault_injection to ruby (grpc#26030) * Backport several additions / fixes to PHP / Ruby xDS Interop tests (grpc#26037) * PHP: allow xDS interop client to start RPCs asynchronously (grpc#25696) * PHP: allow xDS interop client to start RPCs asynchronously * Address review comments * Remove adhoc test config * PHP: enable fault_injection xds interop test case (grpc#25943) * PHP: enable fault_injection xds interop test case * sanity check yaph_code.sh * some mysterious SIGTERM is being observed * Remove adhoc test cfg * Ruby: Fix xds fault_injection test (grpc#26006) * Fix header_matching for PHP and Ruby (grpc#26017) * [Backport][1.37.x] Add kokoro job for xds psm security tests (grpc#26035) * Add kokoro job for xds psm security tests (grpc#26033) * add kokoro job for xds psm security * Fix scripts * Cleanup * Add copyright * Use existing repo for test driver (grpc#26041) * Use existing repo for test driver * Reviewer comments * Don't check local certs for xds k8s kokoro job (grpc#26046) * xds k8s kokoro - Configure auth for docker (grpc#26050) * xds k8s kokoro - Configure auth for docker * Add -q * xds k8s kokoro job - Add git commit as tag for images (grpc#26051) * [Backport][1.37.x] test_csds (grpc#26047) * Add CSDS xDS interop test (grpc#26007) * Add CSDS xDS interop test * Add CSDS test to the test suite * Fix a typo * Address comments * Improve the logging of each attempt * Improve Python readability * Fix a typo in CSDS test (grpc#26021) * Update the Python dependency for xDS interop test (grpc#26024) `xds-protos` includes all the generated Python files for Envoy protos, which is required by Envoy. * Upgrade setuptools and ProtoBuf Python in prep_xds.sh (grpc#26029) * Upgrade * Trick Kokoro to run xDS test * Restore grpc_python_bazel_test.cfg * Update protobuf * Add extra layer of protection * [C++] Add admin and reflection to xds interop binaries (grpc#25964) * Add admin and reflection to xds interop binaries * Prepare for grpc#25978 * Remove _xds rules * Import json_format * [Backport][1.37.x] Backport xds-k8s driver changes (grpc#26067) * xds-k8s: Update Private CA GKE workload certificates config (grpc#25875) * xds-k8s: Update GKE workload certificates: fix annotation (grpc#25882) * xds-k8s: Use latest TD bootstrap supporting new secrets dir (grpc#25925) * Naming fix (secrets manager -> secret manager) (grpc#25990) Co-authored-by: Seth Vargo <seth@sethvargo.com> * temporarily change ::createInsecure() back to return NULL (grpc#26054) * buildscripts: xds-k8s pin pip to 21.0.1 (grpc#26088) pip 21.1 released on Apr 24 introduced a regression for python 3.6.1. The regression was identified on Apr 24, the fix merged on Apr 25. The fix is expected to be delivered in the 21.1.1 patch. There's no clear date, when 21.1.1 will be released. Until then, pin is temporarily pinned to the previous release, 21.0.1. * Bump to 1.37.1 (grpc#26040) * Bump to 1.37.1 * Regenerate projects * [Backport][1.37.x] xds-k8s buildscripts sync (grpc#26103) * Python PSM Security Interop Client+Server (grpc#25991) * Add channelz to security client * Make client changes. Some config needs to be reverted * Add missing security field to channelz Socket * Add test * Fix test * Remove whitespaces for windows * Remove local_certificate check from PSM security tests * Byte pack Python channelz IPs * Move address packing to Core * WIP * Unbork security tests * Python interop server * Turn up server logging * Clean up PR * Clean up some more * Yapf * Fix up docker images * Swap fillllllles! * Add more robust boolean arg parsing * Use bool_arg parsing in server as well * Add copyright Co-authored-by: Yash Tibrewal <yashkt@google.com> * ADD Python xDS security interop test CI scripts (grpc#26073) * xds-k8s kokoro buildscripts: exclude from tests suites (grpc#26098) * xds-k8s buildscript should use the latest version of the driver (grpc#26100) Co-authored-by: Richard Belleville <rbellevi@google.com> Co-authored-by: Yash Tibrewal <yashkt@google.com> Co-authored-by: Richard Belleville <rbellevi@google.com> Co-authored-by: Lidi Zheng <lidiz@google.com> Co-authored-by: apolcyn <apolcyn@google.com> Co-authored-by: Stanley Cheung <stanleycheung@google.com> Co-authored-by: Menghan Li <menghanl@google.com> Co-authored-by: Hannah Shi <hannahshisfb@gmail.com> Co-authored-by: Jan Tattermusch <jtattermusch@users.noreply.github.com> Co-authored-by: Doug Fawley <dfawley@google.com> Co-authored-by: Sergii Tkachenko <sergiitk@google.com> Co-authored-by: Seth Vargo <seth@sethvargo.com>

commit acc24152ff1acfaf445427d14c8a052f976e78ee Author: Nicolas Noble <pixel@nobis-crew.org> Date: Fri Sep 24 14:13:49 2021 -0700 Compilation failures after feedback alterations. commit 0a36e7c394dd81a4163818979027b658797c986d Author: nicolasnoble <nicolasnoble@users.noreply.github.com> Date: Fri Sep 24 06:33:45 2021 +0000 Automated change: Fix sanity tests commit 0f4ffb131a07c9e941159f1393eb821775fac44f Author: Nicolas Noble <pixel@nobis-crew.org> Date: Thu Sep 23 23:23:51 2021 -0700 Reformatting. commit 3dc3d546ab8fdc7cda3c35a1273f77eb97444999 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Thu Sep 23 23:16:30 2021 -0700 Auto fix. commit 82cc79f4c1f1b6e64f93cf3cd91c6ed53994b99d Author: Nicolas Noble <nicolasnoble@users.noreply.github.com> Date: Thu Sep 23 23:04:22 2021 -0700 Addressing some comments. commit 8f3f20bcfbbd634621930c77e319165d0d78017a Author: Nicolas Noble <pixel@nobis-crew.org> Date: Wed Sep 1 19:16:59 2021 -0700 More shuffling code around, a fix for Promises, more documentation. commit 233811aa87e2a556daba36df518b51fe7e0ffea3 Author: Nicolas Noble <pixel@nobis-crew.org> Date: Tue Aug 31 16:28:25 2021 -0700 Names cleanup & visibility fixes. commit 430ad6a2fd1dc8f41b66f1dc289f09c59f206d64 Author: Nicolas Noble <pixel@nobis-crew.org> Date: Mon Aug 30 17:08:58 2021 -0700 Regenerating files. commit 2895cb6cf2cbbda710c3806c177d688078688774 Author: Nicolas Noble <pixel@nobis-crew.org> Date: Mon Aug 30 16:49:09 2021 -0700 Cleanup of SliceBuffer + getting tests to build. commit ed2f43beef4df87b9f82b2b2a4835b37afead852 Merge: 76e95f6afd 6b75f7ecc3 Author: Nicolas 'Pixel' Noble <pixel@nobis-crew.org> Date: Mon Aug 30 12:43:47 2021 -0700 Merge remote-tracking branch 'origin/uv-ee' into ee-libuv # Conflicts: # BUILD # BUILD.gn # CMakeLists.txt # Makefile # bazel/grpc_deps.bzl # build_autogenerated.yaml # config.m4 # config.w32 # gRPC-Core.podspec # grpc.gemspec # grpc.gyp # include/grpc/event_engine/endpoint_config.h # include/grpc/event_engine/event_engine.h # include/grpc/event_engine/slice_allocator.h # package.xml # src/core/ext/transport/chttp2/transport/chttp2_slice_allocator.cc # src/core/lib/event_engine/event_engine.cc # src/core/lib/iomgr/event_engine/endpoint.cc # src/core/lib/iomgr/event_engine/endpoint.h # src/core/lib/iomgr/event_engine/iomgr.cc # src/core/lib/iomgr/event_engine/iomgr.h # src/core/lib/iomgr/event_engine/pollset.cc # src/core/lib/iomgr/event_engine/tcp.cc # src/python/grpcio/grpc_core_dependencies.py # test/core/event_engine/BUILD # test/core/http/test_server.py # tools/doxygen/Doxyfile.c++.internal # tools/doxygen/Doxyfile.core.internal # tools/run_tests/generated/tests.json commit 6b75f7ecc39cf7acd00f3d6a15a4b94e3fd070b2 Author: AJ Heller <hork@google.com> Date: Wed Jul 21 13:03:25 2021 -0700 Initial commit of EE::Slice tests commit c90323ae57412bad1d7a848fc9fd3039f3deb16c Author: AJ Heller <hork@google.com> Date: Wed Jul 21 12:50:14 2021 -0700 Add another stubbed out test commit 5577618a6aea7bf25afb4dc18dd443abf17494d1 Author: AJ Heller <hork@google.com> Date: Wed Jul 21 12:47:17 2021 -0700 Initial commit of EventEngine SliceBuffer tests commit 817930c2b7b650f5e74816b96591ea9c6b6b0792 Author: Nicolas Noble <nicolasnoble@users.noreply.github.com> Date: Thu Jul 15 14:07:40 2021 -0700 Removing duplicated global event engine... commit 1910f755080901912765a897dd44b43fda1cafc4 Author: Nicolas Noble <nicolasnoble@users.noreply.github.com> Date: Thu Jul 15 14:05:10 2021 -0700 Adding an ApplicationCallbackExecCtx to the thread commit 54a5aa780056937ff6b8809c6e46faaf45b0ede5 Author: Nicolas Noble <nicolasnoble@users.noreply.github.com> Date: Wed Jul 14 17:48:31 2021 -0700 Slightly better SIGPIPE solution. commit 9c9b512eeb30c426a11555731285cff2cf9bb9c6 Author: Nicolas Noble <nicolasnoble@users.noreply.github.com> Date: Wed Jul 14 17:42:14 2021 -0700 Bandaid for SIGPIPE Apparently this is the official position of libuv. The user has to explicitly ignore SIGPIPE. commit 74f10d1aee7a571af9872b39ec4c37fe9569c8fc Author: Nicolas Noble <nicolasnoble@users.noreply.github.com> Date: Wed Jul 14 17:02:38 2021 -0700 Properly scheduling past deadlines. commit 1762d4c97d3fa54ee649cea5a80980be760c56ec Author: Nicolas Noble <nicolasnoble@users.noreply.github.com> Date: Sun Jul 11 20:12:39 2021 -0700 Properly recalculating "now" commit bedadaf522290c1a768d0b9748cfc0d92f526988 Author: Nicolas Noble <nicolasnoble@users.noreply.github.com> Date: Fri Jul 9 10:49:13 2021 -0700 Preventing double-closes. commit f1c30df0959e158433e1f179698711496400ccac Author: AJ Heller <hork@google.com> Date: Thu Jun 24 16:04:59 2021 -0700 Allow channel_args to be a nullptr. This is a valid use case. commit 27389e6c6e75d3d85834d69806b16eb9556e4446 Merge: 175703e90e 64e329d928 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Tue Jun 22 10:40:16 2021 -0700 Merge branch 'ee-iomgr-impl' of github.com:drfloob/grpc into uv-ee commit 64e329d928d9e61e0e96cbe0df03b1205af9d660 Author: AJ Heller <hork@google.com> Date: Thu Jun 17 16:16:29 2021 -0700 Remove GetResourceUser from the SliceAllocator API commit 7290a83d3a7bb3f34c97173637826562c77b10d2 Author: AJ Heller <hork@google.com> Date: Thu Jun 17 15:41:23 2021 -0700 addressing reviewer feedback commit 61765d715e1877632886727775418ce346f4aeb4 Author: AJ Heller <hork@google.com> Date: Thu Jun 17 12:50:58 2021 -0700 Addressing review feedback commit 5d66f01671b48e2873a26ddbe613de31f15fdd48 Author: AJ Heller <hork@google.com> Date: Wed Jun 16 13:30:53 2021 -0700 Add port_platform to headers commit 7f28c7fe6a4febc8301f5292c6a8ce68f6a5e7f1 Author: AJ Heller <hork@google.com> Date: Wed Jun 16 11:53:43 2021 -0700 Address reviewer comments commit edb2cf01ceba151f833e9a65afab327f67cec7d3 Author: AJ Heller <hork@google.com> Date: Tue Jun 15 19:10:42 2021 -0700 Reviewer comments commit eb1850ae41c5ea629cb2a6641d7205bbd6dc3660 Merge: 239e3869a1 83681f2721 Author: AJ Heller <hork@google.com> Date: Tue Jun 15 12:01:54 2021 -0700 Merge branch 'master' into ee-iomgr-impl commit 239e3869a1f8ff026f6bb354b24d7fe1201ff03e Merge: d7ecfd42c0 5cbbf20c06 Author: AJ Heller <hork@google.com> Date: Mon Jun 14 15:00:14 2021 -0700 Merge branch 'master' into ee-iomgr-impl commit d7ecfd42c07e468bdcfa057147db7776c20dd758 Author: AJ Heller <hork@google.com> Date: Fri Jun 11 11:29:12 2021 -0700 Remove std::future from EE resolver code commit 6acbaf0bd5005c71ae489a809ea31c0f41bd4b44 Author: AJ Heller <hork@google.com> Date: Fri Jun 11 10:44:03 2021 -0700 Replace the banned std::future with a minimal Promise impl commit ad905f2c7e2b3ad067ec4eb613fe337ca446a525 Author: AJ Heller <hork@google.com> Date: Fri Jun 11 10:16:24 2021 -0700 Cleanup TODOs, add documentation, remove redundant comments commit ea6687db755bc358d11937057b4cba2f3089c20d Author: AJ Heller <hork@google.com> Date: Thu Jun 10 13:57:35 2021 -0700 Remove unneeded(?) gtest clang tidy complained gtest wasn't found, but that seems like a false alarm. commit 7bd10ab2440646a338f5f676725a5de8cdd25db9 Merge: 9021aa0fe7 6744e8f84a Author: AJ Heller <hork@google.com> Date: Thu Jun 10 13:56:34 2021 -0700 Merge branch 'master' into ee-iomgr-impl commit 9021aa0fe7a657ec01a4a23cbca078cb365942c7 Author: AJ Heller <hork@google.com> Date: Thu Jun 10 13:33:34 2021 -0700 Tidy commit f75428f8de0327d630d87e78ad9ae7ae59c55d55 Author: AJ Heller <hork@google.com> Date: Thu Jun 10 11:26:42 2021 -0700 Sanitize commit fbb528ba58dbb90cde349fbaee956028506420fe Author: AJ Heller <hork@google.com> Date: Thu Jun 10 10:39:00 2021 -0700 Rename the server-specific grpc_endpoint create method for EventEngine commit a1c37df98fa5825ce8b68ba3277223b1ba3fb395 Author: AJ Heller <hork@google.com> Date: Wed Jun 9 19:39:51 2021 -0700 Experiment: conditionally replace ExecCtx::Run with EventEngine->Run commit bfbd28df5bf3878305135ae72931fe547014f6de Author: AJ Heller <hork@google.com> Date: Wed Jun 9 13:52:05 2021 -0700 Fix incorrect variable name commit 8db59c475d440ee4ebf3d6be23726c04e64ef55e Author: AJ Heller <hork@google.com> Date: Wed Jun 9 13:48:54 2021 -0700 Pass ChannelArgsEndpointConfig through iomgr to the EventEngine commit 45483d76de4e97c6137b66fe3b2b323a5961b0ad Author: AJ Heller <hork@google.com> Date: Wed Jun 9 13:17:52 2021 -0700 Constify grpc_channel_args in EndpointConfig commit 86e9815fe4d42271a05926d51bfdcce455469c16 Author: AJ Heller <hork@google.com> Date: Mon Jun 7 16:10:35 2021 -0700 Reimplement EndpointConfig based on grpc_channel_args We now expose a simple EndpointConfig interface that speaks absl::variant, and our internal implementation is a thin readonly wrapper around grpc_channel_args. commit 03757ec1256a8faaa0f4bc8d36c83dff2dcc6d78 Merge: 373cd1ac72 e38e59320c Author: AJ Heller <hork@google.com> Date: Fri Jun 4 13:29:30 2021 -0700 Merge branch 'master' into ee-iomgr-impl commit 373cd1ac720df5f62b22b2a896050c8f2dba710c Author: AJ Heller <hork@google.com> Date: Fri Jun 4 13:29:25 2021 -0700 cleanup commit 17d7cc15d72e3e24b851285daa7ea5263b98c122 Author: AJ Heller <hork@google.com> Date: Fri Jun 4 12:40:28 2021 -0700 Sanitize and tidy commit 60d580e68c32c9ee204d3e4cb317acead2153d94 Author: AJ Heller <hork@google.com> Date: Fri Jun 4 11:57:36 2021 -0700 Attempt to fix e2e tests for default non-EventEngine builds commit 06e509d7087cab3ca1519257f5b8a9a778073263 Author: AJ Heller <hork@google.com> Date: Fri Jun 4 11:35:32 2021 -0700 Add mechanism to enable EventEngine-iomgr per platform It is not enabled by default on any platform to begin with. Enabling it requires a change to port_platform.h. commit 6ca3f9a3c77eec0a591feac8007e49f2ef3a83eb Author: AJ Heller <hork@google.com> Date: Thu Jun 3 12:58:57 2021 -0700 Add EE::IsValidEndpointConfig, EndpointConfig::at; make methods const; add BoolType commit 33db243e4b19ea38f82a71c27e503a99d71dd910 Author: AJ Heller <hork@google.com> Date: Thu Jun 3 12:06:19 2021 -0700 Make variant types explicitly constructed; move trace flag commit 27a4f9883488477c7f9fdec55506bb800a270048 Author: AJ Heller <hork@google.com> Date: Wed Jun 2 20:27:31 2021 -0700 Implement EndpointConfig commit 24505323d649d3d05b9771383d1db421e86632db Author: AJ Heller <hork@google.com> Date: Wed Jun 2 16:36:26 2021 -0700 Add iomgr & EventEngine API changes from drfloob/pull/1 @nicolasnoble and @drfloob commit f3a600f4f75de31aa381c8299c82fef0715d1ed4 Merge: 8600c357fa 2857e70d5d Author: AJ Heller <hork@google.com> Date: Wed Jun 2 10:31:58 2021 -0700 Merge branch 'master' into ee-iomgr-impl commit 175703e90edf37591a921cf45c0164fe70b0b9e5 Author: AJ Heller <hork@google.com> Date: Thu May 27 15:25:53 2021 -0700 Fix shutdown issues? There was no signal to stop uv_run from trying again. commit 72c12483686fbad10cb51a3871429fd28321f6ad Author: AJ Heller <hork@google.com> Date: Thu May 27 13:21:43 2021 -0700 Hide uv-ee's grpc_polling_trace definition from non-EE builds Necessary if we want to run non-EE tests from this branch commit 1f4b06d369fe9ec657022658e4bab9787a1b9881 Merge: c7f662927c 190348e8b4 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Thu May 27 09:26:33 2021 -0700 Merge branch 'uv-ee-contrib' of github.com:drfloob/grpc into uv-ee commit 190348e8b491cd3a3a6ccef7b4e27d782af454a4 Author: AJ Heller <hork@google.com> Date: Wed May 26 16:04:05 2021 -0700 Adding to a pollset should be a no-op commit b22510655c47c6cd289fa7780a8f78ff295f6f4f Author: AJ Heller <hork@google.com> Date: Wed May 26 15:51:04 2021 -0700 Integrate the SliceAllocator with uvEngine commit c7f662927c340a888b5a70c5cdac30f273e8dcf8 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Wed May 26 15:44:25 2021 -0700 More debug logs, proper shutdown sequence using iomgr_platform_is_any_background_poller_thread properly. commit b478e146668924eb99d068c093c5591a184a4a7a Merge: 763531c6ab 62cc644350 Author: AJ Heller <hork@google.com> Date: Wed May 26 15:36:45 2021 -0700 Merge branch 'uv-ee' into uv-ee-contrib commit 62cc644350960e6729053485b29b1865053cbc56 Merge: 52ab5e39ec 1d3462a198 Author: AJ Heller <hork@google.com> Date: Wed May 26 15:30:02 2021 -0700 Merge branch 'uv-ee' of github.com:nicolasnoble/grpc into uv-ee commit 1d3462a198bebb9aa4a4d7ce1d665a67332cd12c Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Wed May 26 11:37:31 2021 -0700 Properly calling shutdown callbacks from listeners. commit 98d073fb24f7fc1e1896d7df653969871ab98763 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Wed May 26 11:23:07 2021 -0700 Yet another hexdump pretty fix. commit cbaf05eb338da5935c2a078d47fd583d03a9c3b6 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Wed May 26 11:16:18 2021 -0700 Cleaned up some status code, and cleaned up hostname lookup. commit 800b0153a75d53e65471ff229f67612e75355290 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Wed May 26 10:29:19 2021 -0700 Better handling of cancellations. commit 763531c6abb44019603c181afd5a47b662fc0240 Merge: 2c3e753182 52ab5e39ec Author: AJ Heller <hork@google.com> Date: Wed May 26 10:16:09 2021 -0700 Merge branch 'uv-ee' into uv-ee-contrib commit 52ab5e39ecf42b58bec99f2223b50e3a6f38abda Merge: c2c98d16cf 4c9ad85f4e Author: AJ Heller <hork@google.com> Date: Wed May 26 09:45:18 2021 -0700 Merge branch 'uv-ee' of github.com:nicolasnoble/grpc into uv-ee commit 4c9ad85f4ecb6c950009031066dc6c61edabd9a7 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Tue May 25 19:38:05 2021 -0700 Properly translating canceled status. commit 2c3e753182fb262eb5c4635b8cdc96ee670fbf75 Author: AJ Heller <hork@google.com> Date: Tue May 25 18:58:49 2021 -0700 UNTESTED: first draft of SliceAllocator commit c2c98d16cf326cbbc162c140a416b36511fb0ffe Merge: 2e29c21b2e e6ec175f39 Author: AJ Heller <hork@google.com> Date: Tue May 25 14:56:35 2021 -0700 Merge branch 'uv-ee' of github.com:nicolasnoble/grpc into uv-ee commit e6ec175f39220a38898f87bd86393c7cc57abc4f Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Tue May 25 14:20:57 2021 -0700 Straightening logs a bit, and removing another 'this' capture. commit 8600c357fa2b6adffe0b120e1f499baa28916b5f Author: AJ Heller <hork@google.com> Date: Tue May 25 09:52:10 2021 -0700 Import Nico's iomgr-specific work from the uv-ee branch commit 2e29c21b2e9cc6b875ee9aa4bc96d5b5cdf4b87e Merge: b80e94c7e2 f046b23112 Author: AJ Heller <hork@google.com> Date: Tue May 25 09:28:16 2021 -0700 Merge branch 'ee-iomgr-impl' into uv-ee commit f046b2311236e0c3e09d1adee43d45a535daf180 Merge: 39ee83de34 edae7450f5 Author: AJ Heller <hork@google.com> Date: Mon May 24 17:32:59 2021 -0700 Merge branch 'master' into ee-iomgr-impl commit b80e94c7e27ec5672251fc9c0738e9954274a8e8 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Mon May 24 16:53:14 2021 -0700 Properly referencing input slice buffer in ::Write commit e462d292bed707638348e3fae7d48bfe3d4a2a5e Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Thu May 20 15:13:27 2021 -0700 Almost. commit 2a5625015b84c4c795f227747dd5412c531c5b72 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Thu May 20 15:10:45 2021 -0700 Fixing hexdump view. commit f13133cf2e53f7c0ff26d1d6dea3a43299cacd1c Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Thu May 20 13:07:44 2021 -0700 Band aid for SO_REUSEPORT. commit 19cd590eb0eea53779ef8982e896cff9a398bc75 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Thu May 20 10:15:03 2021 -0700 Adding bind logging. commit 6d0047fb5c30ecc6dd68990c92cf3d22cf8bc92c Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Wed May 19 17:38:57 2021 -0700 More cleanup, and adding some tcp traces. commit c200ec4b42d286e8fc0b643f9c76796f9a84fecc Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Wed May 19 15:14:09 2021 -0700 Shutdown sequence. commit d80aab884fbfdd39877d0a11350739c84221aa25 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Wed May 19 11:14:19 2021 -0700 Folding / moving libuv callbacks when possible / applicable. commit 5818c534d2a672c14ed224ff6d8bedf12d4a348f Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Wed May 19 10:44:51 2021 -0700 Better ownership of libuv resources. commit 39ee83de3429f8b48baebef981da7657e5fb9688 Author: AJ Heller <hork@google.com> Date: Fri May 14 15:07:23 2021 -0700 Test: allow all EventEngine code in the podspec Compiling the EventEngine C++ code was failing weeks ago, but I cannot reproduce locally. Adding it back to see if the Kokoro build succeeds. commit 2426117b871ac9d33b324b6d3c03b3754a05540b Author: AJ Heller <hork@google.com> Date: Thu May 13 15:17:05 2021 -0700 Fix the grpc_endpoint buffer types commit 00e62dd9fb4cfce67ddfc90ac01425499ea09008 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Thu May 13 15:12:38 2021 -0700 End2end tests working? commit f3cf27f017332b3d7ad17fb4ef2f3e19ec940679 Merge: 51ad88d186 67940dcd3b Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Thu May 13 13:40:04 2021 -0700 Merge branch 'ee-iomgr-impl' of github.com:drfloob/grpc into uv-ee commit 67940dcd3b20009efcd0ff1b0e3b3b7c3002783f Author: AJ Heller <hork@google.com> Date: Wed May 12 15:32:24 2021 -0700 Add another event_engine file to the ObjC build - not sustainable ObjC needs a more complete solution for EventEngine integration since it cannot compile C++ code as things stand today. commit c3b68107643eb3b01d67c7e5bcd68887ef3ee00d Author: AJ Heller <hork@google.com> Date: Thu May 13 10:28:23 2021 -0700 Add ability to get the internal grpc_resource_user from an Endpoint commit afa95f927648b4e8a185eccb0b7f492d70088540 Author: AJ Heller <hork@google.com> Date: Wed May 12 12:49:44 2021 -0700 Sanitize commit 151f58efd863af656979baf1ce3d03d37374990e Merge: d6fc4cb3ba 98ccb7fd94 Author: AJ Heller <hork@google.com> Date: Wed May 12 09:24:08 2021 -0700 Merge branch 'master' into ee-iomgr-impl commit 51ad88d186f6a225d3206a5059aaf4afc29dd330 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Tue May 11 16:45:46 2021 -0700 Filling the StartListen, and addresses. commit 6872399724b1b556300085df568b28788d5f032d Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Tue May 11 15:58:02 2021 -0700 Allowing empty and copy constructions of ResolvedAddress. commit a66ff3657a976ed65eba3ddbe47e1fbe2a5e2234 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Tue May 11 13:46:10 2021 -0700 Quick implementation for Run and RunAt. commit a0ed65fa81c44e6718ad6b5bd3d385a94b78f79a Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Tue May 11 13:44:23 2021 -0700 Fixing resolver by moving away from gpr_event to std::promise. commit 9634edfe2bd33e548686fbfc36eb392d64f5820d Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Mon May 10 16:12:23 2021 -0700 Turns out we need the base constructor. commit fc423d139bc82474b070b4a6a9793bf3a220dd54 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Mon May 10 16:09:53 2021 -0700 Removing debug messages. commit 3a45322b5068a09dce14e9a3f1c15e9fa3344633 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Mon May 10 15:55:10 2021 -0700 Proper shutdown sequence, and removing resolver bandaid. commit d6fc4cb3bae903c93afe45bc45ef1cd2ca6374f5 Author: AJ Heller <hork@google.com> Date: Mon May 10 15:47:41 2021 -0700 Disable c-ares for EventEngine The GRPC_ARES macro enables c-ares-based DNS resolution by hijacking the iomgr resolution vtable on gRPC plugin init, overwriting any resolution vtable that may have been set already. We may want to use c-ares for the EventEngine implementation (almost certainly will), but we will not want to use this mechanism to enable it. commit 5696344bffdfce3f4798de5213c32e04c8b40b39 Author: AJ Heller <hork@google.com> Date: Mon May 10 15:46:54 2021 -0700 Delete the correct endpoint commit c741ce28d9777ec212257da2e5c1d80a4e863e50 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Mon May 10 13:32:35 2021 -0700 Fixing address resolving bandaid. commit a8a5d2e9af68897da182308ba029816558d04800 Author: AJ Heller <hork@google.com> Date: Fri May 7 18:44:39 2021 -0700 Bring new EE API callback changes into uv impl commit db963badaf1af875d81f234c524e02a2629105db Author: AJ Heller <hork@google.com> Date: Fri May 7 18:33:31 2021 -0700 Revert "Address reviewer feedback" This reverts commit a54f6b6b468b0b70889d20f94826f191ab1570ee. commit 76f9ec9cf8391e261adf038725f22c1555bc231b Author: Yash Tibrewal <yashkt@google.com> Date: Thu May 6 10:35:54 2021 -0700 Upmerge v1.37.x (#26186) * Implement FilterChainMatch algorithm (#25757) (#25815) * Implement FilterChainMatch logic * Add tests for transport protocol too * Tests for duplicate NACKing * Introduce ConnectionManager as an interface for config fetchers * Do not parameterize IncrementIfNonZero * Some formatting * Reviewer comments * Add filter chain match information for duplicate match error * Reviewer comments * Some cleanup * Reviewer comments * Reviewer comments * Reviewer comments * Clang-tidy * Bump to 1.37.0-pre1 (#25839) * Bump to v1.37.0-pre1 * Regenerate projects * Use Realtime instead of Monotonic time for CSDS (#25864) * Add ruby 3.0 support for mac binary packages (#25869) * Bump to 1.37.0 for Final Release (#25891) * Bump version to 1.37.0 * Regenerate projects * PHP: Fix windows build (#25904) * [Backport] xdsinterop: extend the ports to use (#25916) This is to add more ports for forwarding-rule. It's in theory not necessary, because forwarding-rule doesn't need to use the same port as the services. This is a limitation of the test framework, and can be fixed in the future. * try/catch exceptions of both php7 and php8 (#25918) (#25927) * Fix use-after-unref bug in fault_injection_filter (#25903) (#25935) Co-authored-by: Yash Tibrewal <yashkt@google.com> * Fix fault injection filter to still run the original trailing metadata closure when no error (#25933) * also build python3.6 aarch64 manylinux2014 wheel (#25944) * Enable channelz for xds_interop_client and xds_interop_server (#25939) (#25968) * Enable channelz for xds_interop_client and xds_interop_server * Regenerate projects * Fix #25897 to avoid crashes when certificates are not yet updated (#25899) (#25965) * xds: move path_matching and header_matching to all; add fault_injection to ruby (#26030) * Backport several additions / fixes to PHP / Ruby xDS Interop tests (#26037) * PHP: allow xDS interop client to start RPCs asynchronously (#25696) * PHP: allow xDS interop client to start RPCs asynchronously * Address review comments * Remove adhoc test config * PHP: enable fault_injection xds interop test case (#25943) * PHP: enable fault_injection xds interop test case * sanity check yaph_code.sh * some mysterious SIGTERM is being observed * Remove adhoc test cfg * Ruby: Fix xds fault_injection test (#26006) * Fix header_matching for PHP and Ruby (#26017) * [Backport][1.37.x] Add kokoro job for xds psm security tests (#26035) * Add kokoro job for xds psm security tests (#26033) * add kokoro job for xds psm security * Fix scripts * Cleanup * Add copyright * Use existing repo for test driver (#26041) * Use existing repo for test driver * Reviewer comments * Don't check local certs for xds k8s kokoro job (#26046) * xds k8s kokoro - Configure auth for docker (#26050) * xds k8s kokoro - Configure auth for docker * Add -q * xds k8s kokoro job - Add git commit as tag for images (#26051) * [Backport][1.37.x] test_csds (#26047) * Add CSDS xDS interop test (#26007) * Add CSDS xDS interop test * Add CSDS test to the test suite * Fix a typo * Address comments * Improve the logging of each attempt * Improve Python readability * Fix a typo in CSDS test (#26021) * Update the Python dependency for xDS interop test (#26024) `xds-protos` includes all the generated Python files for Envoy protos, which is required by Envoy. * Upgrade setuptools and ProtoBuf Python in prep_xds.sh (#26029) * Upgrade * Trick Kokoro to run xDS test * Restore grpc_python_bazel_test.cfg * Update protobuf * Add extra layer of protection * [C++] Add admin and reflection to xds interop binaries (#25964) * Add admin and reflection to xds interop binaries * Prepare for https://github.com/grpc/grpc/pull/25978 * Remove _xds rules * Import json_format * [Backport][1.37.x] Backport xds-k8s driver changes (#26067) * xds-k8s: Update Private CA GKE workload certificates config (#25875) * xds-k8s: Update GKE workload certificates: fix annotation (#25882) * xds-k8s: Use latest TD bootstrap supporting new secrets dir (#25925) * Naming fix (secrets manager -> secret manager) (#25990) Co-authored-by: Seth Vargo <seth@sethvargo.com> * temporarily change ::createInsecure() back to return NULL (#26054) * buildscripts: xds-k8s pin pip to 21.0.1 (#26088) pip 21.1 released on Apr 24 introduced a regression for python 3.6.1. The regression was identified on Apr 24, the fix merged on Apr 25. The fix is expected to be delivered in the 21.1.1 patch. There's no clear date, when 21.1.1 will be released. Until then, pin is temporarily pinned to the previous release, 21.0.1. * Bump to 1.37.1 (#26040) * Bump to 1.37.1 * Regenerate projects * [Backport][1.37.x] xds-k8s buildscripts sync (#26103) * Python PSM Security Interop Client+Server (#25991) * Add channelz to security client * Make client changes. Some config needs to be reverted * Add missing security field to channelz Socket * Add test * Fix test * Remove whitespaces for windows * Remove local_certificate check from PSM security tests * Byte pack Python channelz IPs * Move address packing to Core * WIP * Unbork security tests * Python interop server * Turn up server logging * Clean up PR * Clean up some more * Yapf * Fix up docker images * Swap fillllllles! * Add more robust boolean arg parsing * Use bool_arg parsing in server as well * Add copyright Co-authored-by: Yash Tibrewal <yashkt@google.com> * ADD Python xDS security interop test CI scripts (#26073) * xds-k8s kokoro buildscripts: exclude from tests suites (#26098) * xds-k8s buildscript should use the latest version of the driver (#26100) Co-authored-by: Richard Belleville <rbellevi@google.com> Co-authored-by: Yash Tibrewal <yashkt@google.com> Co-authored-by: Richard Belleville <rbellevi@google.com> Co-authored-by: Lidi Zheng <lidiz@google.com> Co-authored-by: apolcyn <apolcyn@google.com> Co-authored-by: Stanley Cheung <stanleycheung@google.com> Co-authored-by: Menghan Li <menghanl@google.com> Co-authored-by: Hannah Shi <hannahshisfb@gmail.com> Co-authored-by: Jan Tattermusch <jtattermusch@users.noreply.github.com> Co-authored-by: Doug Fawley <dfawley@google.com> Co-authored-by: Sergii Tkachenko <sergiitk@google.com> Co-authored-by: Seth Vargo <seth@sethvargo.com> commit a54f6b6b468b0b70889d20f94826f191ab1570ee Author: AJ Heller <hork@google.com> Date: Thu May 6 14:14:52 2021 -0700 Address reviewer feedback commit b410c5b818cbbfbb0725bd4b9ab20c40e6f2374c Merge: e7488aab36 ad7d06364f Author: AJ Heller <hork@google.com> Date: Fri May 7 18:28:58 2021 -0700 Merge branch 'ee-iomgr-impl' into uv-ee commit ad7d06364ff9b2e396c1350bb273bb6122f29ee1 Author: AJ Heller <hork@google.com> Date: Fri May 7 18:01:52 2021 -0700 Incorporate API changes, merging args into StatusOr<...> commit 852106b5bb59121e7bf2af4f035f3044def65d24 Author: AJ Heller <hork@google.com> Date: Fri May 7 18:00:25 2021 -0700 Clean up a few unused arguments commit 634b131af41d8ef9ab697ab6503a229a725d5af2 Author: AJ Heller <hork@google.com> Date: Fri May 7 17:25:46 2021 -0700 Mark move constructors as noexcept See clang tidy performance-noexcept-move-constructor commit 416ee1ebb598e37690c39f7afcf39f2fd387178a Author: AJ Heller <hork@google.com> Date: Fri May 7 13:42:20 2021 -0700 Move EventEngine-iomgr build target back into grpc_base_c commit d42441e7260376e70654e0a81695f4915e8f0927 Merge: 9b68df6606 5b035265ce Author: AJ Heller <hork@google.com> Date: Fri May 7 12:47:17 2021 -0700 Merge branch 'master' into ee-iomgr-impl commit 9b68df66060500262bb37ead7cfcfc7ebb46263b Author: AJ Heller <hork@google.com> Date: Thu May 6 14:14:52 2021 -0700 Address reviewer feedback commit e7488aab36b72c7f482cf566520a4d0b6cad6765 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Wed May 5 18:45:13 2021 -0700 Trying to implement Shutdown, and adding some debug logging for why we're still holding references on shutdown. commit 22d7f00b20f319bd86ece09df5699d681ba104e6 Author: AJ Heller <hork@google.com> Date: Wed May 5 13:27:13 2021 -0700 Remove Status parameter from AcceptCallback CreateListener may fail synchronously, returning a Status. The Listener may fail somehow after it's been started, in which case on_shutdown will be called with a failure Status. This callback will only be called if there's something to accept, and in all is in a good state. commit f2735b2531973ef0e3958b72e50260617dd8ad1b Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Wed May 5 13:02:46 2021 -0700 Better pollset stub. commit a579d366a9fd2ae350d0c65ae4dc1da575127007 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Wed May 5 11:04:32 2021 -0700 Python3 strings fixes, take 2. commit ce305705b949147849c4509ff8fda7a277817d0c Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Wed May 5 10:28:50 2021 -0700 Python3 API change for grabbing headers. commit 0220cfac1830fd4c4ecee6adabdfe84eb521b91d Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Wed May 5 10:24:38 2021 -0700 Python3 strings strike again. commit e3c6861379ed73154a76fc08b011d83cfe9eb0b8 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Wed May 5 10:09:11 2021 -0700 Fixing bug in Read callback, and implementing ~uvEndpoint. commit 628f7ab11c1fac2ee3d8983907b920188e5b25b3 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Wed May 5 10:08:48 2021 -0700 Converting test_server.py to Python3. commit d9d47f2dd53f1be961e1bbaf96bdabb89f300149 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Wed May 5 10:08:02 2021 -0700 Quick bandaid for pollsets in EE. commit f23b9eafde28c4b12f3ce7f2d610be755592108f Merge: f70e3f1057 729d7ee192 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Tue May 4 16:35:53 2021 -0700 Merge branch 'ee-iomgr-impl' of github.com:drfloob/grpc into uv-ee commit f70e3f1057aade81b986a09689df21c20c84f625 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Tue May 4 15:07:42 2021 -0700 libuv reads & writes added. commit 729d7ee1929d350388e82092516a80c82409bb90 Author: AJ Heller <hork@google.com> Date: Tue May 4 15:06:59 2021 -0700 Addressing reviewer feedback commit 727342cb6865e0d77f3eb445cd8a8129c91b4ed0 Author: AJ Heller <hork@google.com> Date: Tue May 4 14:24:48 2021 -0700 Simplify tcp server destruction commit bff8c645202df0d04726154bb4548b18fe448446 Author: AJ Heller <hork@google.com> Date: Tue May 4 14:24:23 2021 -0700 Clean up slice allocator a bit commit 595dd089539e3b8a5182ea6d5db739aa18992f90 Author: AJ Heller <hork@google.com> Date: Tue May 4 13:53:06 2021 -0700 Merge resolved_address_internal with sockaddr_utils commit 70f8c987ce66a08dd09a4d4313801136f2b05a33 Merge: 3180c566ae 7e2a1ba4c5 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Tue May 4 13:42:24 2021 -0700 Merge branch 'ee-iomgr-impl' of github.com:drfloob/grpc into uv-ee commit 7e2a1ba4c5151ec3238b21a2ee6981057882c501 Author: AJ Heller <hork@google.com> Date: Tue May 4 13:37:51 2021 -0700 Add exec_ctx to lambda commit 31d9376310dad804c883fcdc7f0f5f9bfed57bf5 Author: AJ Heller <hork@google.com> Date: Tue May 4 13:37:35 2021 -0700 Fix imports after merging master (iomgr->address_utils) commit 04c5c2ba791a52e5c8b2b106ff2391322b596c3d Merge: 076a9a9d8f 9896eda456 Author: AJ Heller <hork@google.com> Date: Tue May 4 13:20:17 2021 -0700 Merge branch 'master' into ee-iomgr-impl commit 076a9a9d8fa67a4d56fadc2584768bd60841f9d2 Author: AJ Heller <hork@google.com> Date: Tue May 4 12:57:16 2021 -0700 Fix implicit error expectation, v2 commit 3180c566aea8df2e3240e8c28a916b3968cc05cb Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Tue May 4 10:28:19 2021 -0700 Fixing implicit error expectation. commit fba65bd5a75aa477e7bbe0084b78ca4df36c38d8 Author: AJ Heller <hork@google.com> Date: Mon May 3 18:18:34 2021 -0700 Implement tcp server - UNTESTED commit 70dc55a7a3fcc35accbf38d2c52a3f7782cf8844 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Mon May 3 17:01:46 2021 -0700 Dropping SliceBuffer & Slice declarations. commit 63ea9aee0d21da49c3d203acaa1ed134bab4ba6d Author: AJ Heller <hork@google.com> Date: Mon May 3 16:59:48 2021 -0700 The Write API should write things commit 12156fb8700a0291e573e33a0214ef356a2b73be Author: AJ Heller <hork@google.com> Date: Mon May 3 16:11:49 2021 -0700 Store a TaskHandle in grpc_timer commit 1c3a946a3c667c5694d9d5259f8446fa3027b238 Merge: 324e7074cd 0b452b058f Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Mon May 3 15:33:44 2021 -0700 Merge branch 'ee-iomgr-impl' of github.com:drfloob/grpc into uv-ee commit 0b452b058fbf27c383577b7f40db1aa22004ba33 Author: AJ Heller <hork@google.com> Date: Mon May 3 15:30:06 2021 -0700 Implement endpoint read & write; add minimal code for pollsets commit 324e7074cd2cce189a05a47a961ce7303a392b02 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Mon May 3 14:20:10 2021 -0700 Proper async lookup in libuv. commit 0bdd51fbb57c296dddbd5522db71549112d5c2a5 Author: AJ Heller <hork@google.com> Date: Fri Apr 30 14:12:31 2021 -0700 Replace gpr_mu with grpc_core::Mutex in tcp.cc commit 4ac8c47d2b01960c52efe6d1bff1053b88efa5af Author: AJ Heller <hork@google.com> Date: Fri Apr 30 13:55:29 2021 -0700 Replace gpr_refcount with grpc_core::RefCount in tcp.cc commit fdf6b5941ff341f082ce85188c5d6506ac49fe5c Author: AJ Heller <hork@google.com> Date: Fri Apr 30 09:23:09 2021 -0700 Remove grpc_polling_trace (was relocated in the uv-impl branch) commit 6102f0a80763849ab031b40fab592c3b2250ca08 Author: AJ Heller <hork@google.com> Date: Fri Apr 30 08:49:24 2021 -0700 Add default grpc_polling_trace back to event_engine.cc commit 0ae400b07c9dab86b8df14189ca834f82de20729 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Thu Apr 29 14:25:12 2021 -0700 TCP connect in libuv. commit b919011d376035a4f29d81f0dab59909953bc693 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Thu Apr 29 14:24:52 2021 -0700 Fixing grpc_resolve_address kludge. commit 932032781ae1e09b7fe13cd9f9e731554792f994 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Thu Apr 29 14:23:30 2021 -0700 Fixing OnConnectCallback lambda. commit 44797a0702dbbb3049557f1aa858a63c27b5ad26 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Thu Apr 29 14:22:50 2021 -0700 Fixing merge. commit 66e2403c2341e58cc95b7f6c4e55727ddc77c4fa Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Thu Apr 29 13:57:16 2021 -0700 Fixing slice allocator and slice allocator factory. commit 9163357486e0288fdeea615c6e9fdcfb7e603cfd Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Thu Apr 29 13:57:16 2021 -0700 Fixing slice allocator and slice allocator factory. commit aa21cb2669d59085c8e13a3739195d96b849cca1 Merge: 97600e6131 4fafcd2149 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Thu Apr 29 12:51:35 2021 -0700 Merge branch 'ee-iomgr-impl' of github.com:drfloob/grpc into uv-ee commit 4fafcd2149056ab0db643112f5bd55cc1d8b77fd Author: AJ Heller <hork@google.com> Date: Wed Apr 28 20:44:07 2021 -0700 Clang format commit ad9aef18ad07e808f55f38ea52a559fd142008ed Author: AJ Heller <hork@google.com> Date: Wed Apr 28 20:28:00 2021 -0700 Implement blocking resolve in terms of async resolve commit df12b10ca0607d16cf3d57e2506c2037ee8c69be Author: AJ Heller <hork@google.com> Date: Wed Apr 28 19:48:01 2021 -0700 Implement EventEngine-iomgr async resolve commit 8180abf17bcb9f1f8fe5ce5c26dcae98281d911f Author: AJ Heller <hork@google.com> Date: Wed Apr 28 18:13:15 2021 -0700 Cleanup commit e6d85730f83f31a501ee51e391462fd66e427874 Author: AJ Heller <hork@google.com> Date: Wed Apr 28 18:07:03 2021 -0700 Create a separate BUILD target grpc_event_engine_iomgr This is the temporary part of EventEngine-iomgr only, and will eventually be removed. Separating it out helps us keep an eye on dependencies, and makes it a bit easier to clean up later. It seems grpc_error may stick around past iomgr's lifetime, and I'm not sure about WorkSerializer. commit 09764d155e00aa2a15926753bd2852500ae484a5 Author: AJ Heller <hork@google.com> Date: Wed Apr 28 17:08:47 2021 -0700 Ensure all EE cbs create an ExecCtx before running the grpc_closure commit 05e1aebc5cca5de08e9c5d58e1120002112c392f Author: AJ Heller <hork@google.com> Date: Wed Apr 28 14:50:31 2021 -0700 On connection failures, execute callbacks with the returned error commit 4cfff885cba02a7d149fa5cc0759fb5272f231cc Author: AJ Heller <hork@google.com> Date: Wed Apr 28 14:49:23 2021 -0700 Cause iomgr shutdown to block and wait on EE shutdown This currently waits forever. Adding a timeout might be wise. commit 697a03cab04650ec51566b935e06203084b887a2 Author: AJ Heller <hork@google.com> Date: Wed Apr 28 13:51:28 2021 -0700 Remove TODOs for adding mutexen for endpoint shutdown/destroy commit 470d20b5b6ecfbe3ed56ff014319fe101699a383 Author: AJ Heller <hork@google.com> Date: Wed Apr 28 13:42:45 2021 -0700 Move shared_ptrs to spare an atomic inc/dec commit ac5fa73bc31abba4db6482a0a934a8a8ff00e7dc Merge: abe0225b53 2c25e5336a Author: AJ Heller <hork@google.com> Date: Wed Apr 28 13:27:48 2021 -0700 Merge branch 'master' into ee-iomgr-impl commit 97600e613129b66ecc924538e04ced59ec464925 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Wed Apr 28 08:45:37 2021 -0700 Guarding against empty nodes in the multiple producer queue. commit 6eceb165981b0645adbba68e1942ae06a9abf1bf Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Wed Apr 28 08:43:18 2021 -0700 Running the exec context, maybe? commit 3068e796da7b0b3b464496f733bee4cc52e6d894 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Wed Apr 28 08:40:23 2021 -0700 Actually spawning an exec context... but it'd still need to run. commit abe0225b536674f2ff46c16197a0bf7de5363bf6 Author: AJ Heller <hork@google.com> Date: Tue Apr 27 19:19:39 2021 -0700 Flush the existing ExecCtx on tcp_server_unref See http://b/186591397 for investigations into why this should be necessary. My comments from code review > If we don't explicitly flush the ExecCtx, and it doesn't go out of > scope before the server is destroyed, I think we're looking at use after > free issues. commit 62a753d4ee8724e6aa5718bc832fc7bca7826e18 Author: AJ Heller <hork@google.com> Date: Tue Apr 27 18:50:54 2021 -0700 Inline all simple grpc_closure->EventEngine::Closure conversions commit bec56991aef6604b0524d62dcbbc3e85d35fe598 Author: AJ Heller <hork@google.com> Date: Tue Apr 27 18:40:25 2021 -0700 C++-ify grpc_tcp_server's construction/destruction commit d7b16816b0971c5d5d886645ba6bacbbc15265b7 Author: AJ Heller <hork@google.com> Date: Tue Apr 27 18:20:12 2021 -0700 Handle immediate errors on EE::Connect; remove redundant assignments commit 03a058c143673392bba578d90f53a471f80cd0af Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Tue Apr 27 17:46:52 2021 -0700 LookupHostname stubbed. commit 48302db5ee83edcefab94d048522b71096713481 Author: AJ Heller <hork@google.com> Date: Tue Apr 27 17:36:12 2021 -0700 Add post-shutdown logic to endpoint operations commit d9f5815659e70085013ee956037027adb3a433d5 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Tue Apr 27 17:33:58 2021 -0700 Fixing stubbed pollset creation. commit 055c00d4f12d74b94da142f322ca69e2ad13475b Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Tue Apr 27 17:31:50 2021 -0700 Fixing pollset stub - it still requires a mutex to work. commit 45dadf0507f26b125a2bdd9f2e2a6082acd6fc64 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Tue Apr 27 17:31:28 2021 -0700 Fixing constructor of ResolvedAddress commit 84ef9032b6e8bd10f4e0d056b7203c41d209263e Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Tue Apr 27 12:24:01 2021 -0700 Quickly stubbing out grpc_resolve_address to the event engine code. commit 8de067bc804172000d3a3ae4005c47db120fb015 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Tue Apr 27 12:23:20 2021 -0700 Actually starting UV thread. commit 45e568de5352da533491e10ac683b0cc25c98510 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Mon Apr 26 18:31:05 2021 -0700 Simplification: don't dereference the same pointer continously. commit 84284d39abbbb414b5dc295baa50b5dfc203b52a Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Mon Apr 26 18:30:44 2021 -0700 Going with aborts all the way. commit d14aa658d5bb7c5252333345be43973ce4e44ef7 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Mon Apr 26 13:40:13 2021 -0700 Fixes after pulling; tests now build. commit 2fb111175bf32d9a2dd5d4e7705fce202726a2b8 Merge: 5275d80b45 02de7a4e80 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Mon Apr 26 13:14:08 2021 -0700 Merge branch 'ee-iomgr-impl' of github.com:drfloob/grpc into uv-ee commit 02de7a4e80dfc8462c67937d9232405771dc5906 Author: AJ Heller <hork@google.com> Date: Fri Apr 23 17:58:43 2021 -0700 Cache local & peer addresses; Handle RQ refs appropriately on endpoints commit b37a38a3e604da6f2558a1b4977530f94d80ba8a Author: AJ Heller <hork@google.com> Date: Fri Apr 23 17:08:45 2021 -0700 Rename peer_string peer_address; Remove Endpoint::Close commit c3321f2919d4a622ab0707a5d971ba8d130bd8e0 Author: AJ Heller <hork@google.com> Date: Fri Apr 23 12:45:58 2021 -0700 Address reviewer feedback; Remove some unnecessary #ifdef compile guards commit 18c103509b81230301dd6f6effe15e376803bf16 Author: AJ Heller <hork@google.com> Date: Fri Apr 23 10:44:53 2021 -0700 Callbacks take unique_ptr<Endpoint> to make ownership transfer explicit commit 7fb15f2d727076c24075b844f73bdc74b4419c4e Author: AJ Heller <hork@google.com> Date: Thu Apr 22 18:27:38 2021 -0700 Address reviewer feedback Remove some include guards Migrate internal code to the right folders Add EventEngine Shutdown callback etc commit 7cc9f063f64a5fb782085af0450540898147bf33 Author: AJ Heller <hork@google.com> Date: Thu Apr 22 17:11:12 2021 -0700 Fix the build; move network util function defs from util.cc to sockaddr.cc Note that //test/... does not build. Some tests exclude mac and windows, and should exclude EventEngine as well since they assume posix otherwise (see bm_pollset, for example). This also moves commit 5275d80b4521d6ead42f1fbf814a718de26fdc08 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Thu Apr 22 16:49:08 2021 -0700 Using proper Status class. commit 618c45f48f4cb001419da7b60179d79293bbd8a6 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Thu Apr 22 16:18:01 2021 -0700 WIP libUV ee implementation. commit 3fa14a8d62c2a8032105f86cc38d49e6076a1a53 Merge: 5021d6dedd 8777697c68 Author: AJ Heller <hork@google.com> Date: Thu Apr 22 15:04:01 2021 -0700 Merge branch 'master' into ee-iomgr-impl commit 5021d6deddf6582c533f3ad0fff5a555d665f263 Author: AJ Heller <hork@google.com> Date: Thu Apr 22 13:54:46 2021 -0700 Fix endpoint addressing; add base class destructors; define status codes commit e1a570056bd842dfa82861bd9d0a4d7fa4ca91b1 Author: AJ Heller <hork@google.com> Date: Thu Apr 22 09:33:51 2021 -0700 buildifier_format_code and fixes for check_port_platform commit 3953a5321416adb52a253d444ce071611edf99ee Author: AJ Heller <hork@google.com> Date: Wed Apr 21 15:06:00 2021 -0700 EE::ResolvedAddress <-> grpc_resolved_address; Added ToString helper commit 08199f2996a2599d1d6c43bc0f70209ac61283d7 Author: AJ Heller <hork@google.com> Date: Wed Apr 21 14:03:55 2021 -0700 Implement endpoint shutdown & destroy commit 446b448b35cba8884ea5488c2ca1223dd8dd6219 Author: AJ Heller <hork@google.com> Date: Wed Apr 21 10:21:43 2021 -0700 Cleanup Remove redundant static qualifiers in anonymous namespace C++-style copyright Some sane default values A few TODOs commit 00ee003ad3a403a180333b5680e4743d93240c41 Author: AJ Heller <hork@google.com> Date: Tue Apr 20 16:14:56 2021 -0700 EE-iomgr: implement timer Also adds a default value for the TaskHandle key. commit c7422c2af842682e78271c19fa3f4a0754bd15b9 Merge: 950b1cfc0c 5cda00427d Author: AJ Heller <hork@google.com> Date: Tue Apr 20 16:10:01 2021 -0700 Merge branch 'master' into ee-iomgr-impl commit 950b1cfc0c1579d67390d543ca0bdb74afc8aadc Author: AJ Heller <hork@google.com> Date: Tue Apr 20 15:16:52 2021 -0700 Fix sanity checks (port_platform ordering, more compilation guards) commit 9974cb9ae734439e364d4215255edff77af6b027 Author: AJ Heller <hork@google.com> Date: Tue Apr 20 13:21:15 2021 -0700 Eliminate false-positive identification of bad include guards commit 11d73271a9c20bd69d9248b97545a9aee3da4908 Author: AJ Heller <hork@google.com> Date: Tue Apr 20 12:36:56 2021 -0700 Add missing compile guards This is needed for non-EE builds. commit c952eea60920e020b25eed52fcde6947d661f474 Author: AJ Heller <hork@google.com> Date: Tue Apr 20 11:57:17 2021 -0700 Add no-op EventEngine endpoint pair commit 9111a3a1548f388bc9849b4fb786676422b97db2 Author: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org> Date: Tue Apr 20 11:44:38 2021 -0700 Base utility functions, likely not cross platform safe. commit ee4e3d24417ba68e6b00742afdc45362d8d18c0c Author: AJ Heller <hork@google.com> Date: Tue Apr 20 11:36:02 2021 -0700 Run generate_projects.sh commit 5bad273efcb87f3686ee8cf0cc22e01a90c94f0f Merge: 732761dbb7 f03a839f8a Author: AJ Heller <hork@google.com> Date: Tue Apr 20 11:35:09 2021 -0700 Merge branch 'master' into ee-iomgr-impl commit 732761dbb779d65736763b65ad16f3d1029ad9df Author: AJ Heller <hork@google.com> Date: Tue Apr 20 11:33:01 2021 -0700 Add EE-iomgr timer to BUILD; fix some linker errors bazel build --cxxopt='-DGRPC_EVENT_ENGINE_TEST' test/core/end2end:all commit 3dbf01fa3767cf324d2c49ba3b0aed331290d231 Author: AJ Heller <hork@google.com> Date: Mon Apr 19 16:58:57 2021 -0700 Assert non-null ResolvedAddress when creating a grpc_resolved_address commit cca49e5071745133ec03fb20b1c857a2f7c58aa5 Author: AJ Heller <hork@google.com> Date: Mon Apr 19 16:50:03 2021 -0700 EE-iomgr: endpoint peer & local addresses commit e4711692413cedc28a3e05e94e8758781e263ad3 Author: AJ Heller <hork@google.com> Date: Mon Apr 19 12:44:13 2021 -0700 EE-iomgr: implement iomgr shutdown commit 097a53e80fe9b562a365696349b6a8616cf899ef Author: AJ Heller <hork@google.com> Date: Mon Apr 19 12:33:33 2021 -0700 EE-iomgr: implement tcp server ref & unref, destroy, shutdown cbs commit 583f8569cc889cc48ed2860e7cae634a5ee0126e Author: AJ Heller <hork@google.com> Date: Mon Apr 19 11:37:26 2021 -0700 Return port from EE-iomgr tcp_server_add_port commit 4e8127200687c5661fa1642f2a8b7386ce20ed9b Author: AJ Heller <hork@google.com> Date: Mon Apr 19 10:21:00 2021 -0700 Implement EE-iomgr tcp_server_add_port commit cea81d6847452d55b7530e2c40b94c2ee5495877 Merge: 4608f99ff8 84b86fe7e8 Author: AJ Heller <hork@google.com> Date: Mon Apr 19 10:06:13 2021 -0700 Merge branch 'master' into ee-iomgr-impl commit 4608f99ff8dacbcec9b0fa53d1a236931a38c2ba Author: AJ Heller <hork@google.com> Date: Tue Apr 13 17:31:07 2021 -0700 Flesh out tcp_server_create commit ea6dfaa81bafd1c5d7f0ea907cc9d12bba847b00 Author: AJ Heller <hork@google.com> Date: Tue Apr 13 16:35:00 2021 -0700 Move grpc_closure conversion functions closure to their usage commit 99e034c34dbc42ce85ba4eb7de7c640693873650 Author: AJ Heller <hork@google.com> Date: Tue Apr 13 13:26:13 2021 -0700 Flesh out EventEngine-iomgr tcp_connect commit 9a2d5622673effa602c3ce83187193f4ac3b1820 Author: AJ Heller <hork@google.com> Date: Tue Apr 13 13:24:47 2021 -0700 Add EventEngine-iomgr timer stubs commit 0523c6e409caf8d8e3df62073239b9499e47baed Author: AJ Heller <hork@google.com> Date: Fri Apr 9 13:51:51 2021 -0700 Fleshing out endpoint, tcp_client, and closure->fn conversions commit 58ba9a2fec59d7664f48f1c00de12a54bfeb0d9e Merge: 473a1b59dd 7c5a7bd73b Author: AJ Heller <hork@google.com> Date: Thu Apr 8 10:46:16 2021 -0700 Merge branch 'master' into ee-iomgr-impl commit 473a1b59ddd8de11412805b9dd8f224239d94ac4 Merge: 84d03d7c4b 1784e6c962 Author: AJ Heller <hork@google.com> Date: Wed Apr 7 10:41:01 2021 -0700 Merge branch 'master' into ee-iomgr-impl commit 84d03d7c4b4c36c1f474066ed77e223615108649 Merge: 3135e34dcc efe627594e Author: AJ Heller <hork@google.com> Date: Tue Apr 6 09:37:15 2021 -0700 Merge branch 'master' into ee-iomgr-impl commit 3135e34dccceda55545adcce69e08ccbecad87f2 Author: AJ Heller <hork@google.com> Date: Mon Apr 5 13:10:38 2021 -0700 Fix redefinition for ResolvedAddress commit 8261688eca2a7d3d34ed39f9fffeb09f2b38632a Merge: cedf9383df 4d4ee609c1 Author: AJ Heller <hork@google.com> Date: Mon Apr 5 12:44:56 2021 -0700 Merge branch 'master' into ee-iomgr-impl commit cedf9383dffb18df2bacbff9e2cab4f2eba3b22b Author: AJ Heller <hork@google.com> Date: Fri Apr 2 11:25:41 2021 -0700 Fix copyright format (C++ style); Fix one header guard commit 85102f8d07c1e93b23c8e073fb387989a80d1a35 Author: AJ Heller <hork@google.com> Date: Fri Apr 2 11:09:03 2021 -0700 Fix namespace change from the EventEngine interface PR commit 9e9cfcf2e276daae4f77094d19635a449f52807f Merge: 5e37a374a2 2b38fe3e22 Author: AJ Heller <hork@google.com> Date: Fri Apr 2 10:45:56 2021 -0700 Merge branch 'event-engine-interface' into ee-iomgr-impl commit 2b38fe3e220514955c5557d1e4ac95f3f54eecfa Author: AJ Heller <hork@google.com> Date: Fri Apr 2 10:26:07 2021 -0700 Addressing reviewer feedback: README wording, param names, more TODOs commit 5cb11ef87c77643e5140ea7ff93191883925e9bc Merge: 4d20b251a7 fd27fb09b0 Author: AJ Heller <hork@google.com> Date: Fri Apr 2 09:26:33 2021 -0700 Merge branch 'master' into event-engine-interface commit 4d20b251a7e1d6c7fab29878a3ac3af75fdc20eb Author: AJ Heller <hork@google.com> Date: Thu Apr 1 14:25:40 2021 -0700 Addressing reviewer feedback: readme, status TODOs, callback scope commit 4d2a10119b8b41bc90d5fd02c9f2c409f092002e Author: AJ Heller <hork@google.com> Date: Thu Apr 1 12:11:53 2021 -0700 Remove event_engine files from module.modulemap This is used in the Objective-C build. I also cleaned up the template a bit. commit d712291f410d5159b486e4081be3bfecf47dfd97 Author: AJ Heller <hork@google.com> Date: Wed Mar 31 18:02:00 2021 -0700 Fix Objective-C build by excluding EventEngine files Obj-C can't C++ commit bd7e8e3d34d167d61601da7b275ec4c5a68236ee Author: AJ Heller <hork@google.com> Date: Wed Mar 31 15:14:47 2021 -0700 Add event_engine includes to non-core paths for copyright check Using C++-style comments for `#endif // GUARD` commit 900d7828ccd5132773c63091af0b6d83f2fb11e0 Author: AJ Heller <hork@google.com> Date: Wed Mar 31 14:52:27 2021 -0700 Fix copyright format for check_copyright.py commit b1da0817d8970496af6e5f83defdd4e7ad2942b2 Author: AJ Heller <hork@google.com> Date: Wed Mar 31 13:55:18 2021 -0700 clang format commit 4bc39103251749b02bc0b78c2d07902f94203710 Author: AJ Heller <hork@google.com> Date: Wed Mar 31 13:27:39 2021 -0700 reattempt iOS EventEngine compatibility - new EE-specific posix macro This macro may outlive iomgr, whereas the existing one's may not. Eventually, maybe the alts/crypt code can be refactored. commit 450420b8d3aa461678853867bbe85ef89f900e57 Author: AJ Heller <hork@google.com> Date: Wed Mar 31 13:12:18 2021 -0700 reattempt iOS EventEngine compatibility - preprocessor macro fiddling commit bd70500641de253c1da807840896665e70309adc Author: AJ Heller <hork@google.com> Date: Tue Mar 30 13:33:49 2021 -0700 Exclude iOS from posix sockaddr include logic This may be temporary. iOS isn't officially supported by libuv. There is code in libuv for iOS, and a few PRs to make it build. commit 99772bfecf2f3e9c906583e1a24686956d0bbf84 Author: AJ Heller <hork@google.com> Date: Tue Mar 30 13:07:13 2021 -0700 Incorporate platform-specific sockaddr includes in port_platform.h This required modifying code to avoid redefinition of `struct iovec` on platforms that already offer it. commit 5e37a374a22247a9b3c90ece24fb470165c4777f Merge: 03da1fb816 988c1f353c Author: AJ Heller <hork@google.com> Date: Tue Mar 30 12:14:19 2021 -0700 Merge branch 'event-engine-interface' into ee-iomgr-impl commit 988c1f353cc3f917df954280af5f6dffc39f31c4 Author: AJ Heller <hork@google.com> Date: Tue Mar 30 10:29:19 2021 -0700 Carve out exception for event_engine headers in c89 compatibility check commit 0629307609e8df31505cd02db12176f8b92d9740 Author: AJ Heller <hork@google.com> Date: Mon Mar 29 19:07:06 2021 -0700 Addressing reviewer feedback commit d5fe7a896ce7cd884f543371cf2234f43b8bd32b Author: AJ Heller <hork@google.com> Date: Mon Mar 29 18:34:46 2021 -0700 Addressing reviewer feedback commit 03da1fb8169f87d226a8682a921ef1f8876f7b18 Author: AJ Heller <hork@google.com> Date: Mon Mar 29 13:45:36 2021 -0700 Add stubs for an EventEngine-based iomgr implementation commit 52f51a1e312bc80bf7f65b25223684b5426298d0 Author: AJ Heller <hork@google.com> Date: Mon Mar 29 12:45:06 2021 -0700 generate_projects.sh commit 3386086c0fe2d40897b245dea90f54576170bb9b Author: AJ Heller <hork@google.com> Date: Mon Mar 29 12:44:11 2021 -0700 Fix header guard names; Move ResolvedAddress commit 312fbf92147bb69bc280264d718b0eada551bf3c Author: AJ Heller <hork@google.com> Date: Thu Mar 25 14:52:42 2021 -0700 Make EventEngine::DNSResolver API public (missing `public:` keyword) commit 008e4715f3898c290e512013aefd30b59d9bfa46 Author: AJ Heller <hork@google.com> Date: Thu Mar 25 11:04:55 2021 -0700 Add virtual destructors; use internal grpc_resource* methods commit bae8e6e2a2946cf103a66063479bf74b16ec656c Author: AJ Heller <hork@google.com> Date: Wed Mar 24 13:50:48 2021 -0700 Regenrate build files commit 3908ce2afc1ba069244f4c4f30a226f65e8470d1 Merge: 76802be59e 077f627aef Author: AJ Heller <hork@google.com> Date: Wed Mar 24 13:49:35 2021 -0700 Merge branch 'master' into event-engine-interface commit 76802be59e39c13b86e5e636ba3dde2fdcd79444 Author: AJ Heller <hork@google.com> Date: Wed Mar 24 13:47:57 2021 -0700 Move event_engine headers to include/grpc; experimental ns everywhere commit 63febf62760a172c0151ae016fe83c6a7eec7fa9 Author: AJ Heller <hork@google.com> Date: Tue Mar 23 22:55:29 2021 -0700 Add mention of endpoint addr lifetime; remove disparaging language commit 3ceeb26a21b63cbc1b6432cf7eff1badcbba7caa Author: AJ Heller <hork@google.com> Date: Tue Mar 23 22:33:32 2021 -0700 fix clang tidy issues commit 33f3561da3b46fd3781b742ecf16dbe77eed732b Author: AJ Heller <hork@google.com> Date: Tue Mar 23 22:16:54 2021 -0700 Add basic usage examples and fix unused variable warnings commit 566fd919b9ca2e254e05904c2c92222fabb14218 Author: AJ Heller <hork@google.com> Date: Tue Mar 23 21:40:56 2021 -0700 fix grpc_resource_* forward declares commit 593581572c03d0845d0a3111f792548b7b5ed10e Author: AJ Heller <hork@google.com> Date: Tue Mar 23 21:27:16 2021 -0700 Impl: SliceAllocator & SliceAllocatorFactory commit 47cf6a59eee64cbd2fc59a19fe0a189b7c1c358b Merge: 69d0471443 8fcc341b10 Author: AJ Heller <hork@google.com> Date: Tue Mar 23 15:33:11 2021 -0700 Merge branch 'master' into event-engine-interface commit 69d0471443fc36d899b9fe2aea8d9249bb042e09 Author: AJ Heller <hork@google.com> Date: Tue Mar 23 15:26:02 2021 -0700 Replace non-conforming preprocessor guards commit 1c42d5d1f518781af37a212b5e5d4a269c0e3fe6 Author: AJ Heller <hork@google.com> Date: Tue Mar 23 15:02:25 2021 -0700 Run generate_projects.sh; remove meaningless const qualification commit 5115c6d9120c37230ff9c7a72e810951b2ae8b61 Author: AJ Heller <hork@google.com> Date: Tue Mar 23 13:50:35 2021 -0700 Move EventEngine to experimental namespace; add channel arg flag commit ba0be6a7b0ef91909c50c0db731f3d341216c6ee Author: AJ Heller <hork@google.com> Date: Mon Mar 22 14:48:16 2021 -0700 Change header guard macro format commit c811b1751f864f5c24a2aa55f460cca484ef36dd Author: AJ Heller <hork@google.com> Date: Mon Mar 22 14:43:25 2021 -0700 Add EventEngine README stub commit 3173c1a0dd58af66330a15cb5dd718f27e76b52b Author: AJ Heller <hork@google.com> Date: Mon Mar 22 14:31:37 2021 -0700 EventEngine Interface - incomplete first draft

yashykt force-pushed the filterchainmatch branch from c7873a2 to 8b2a0c4 Compare March 18, 2021 17:34

yashykt changed the title ~~Filterchainmatch~~ Implement FilterChainMatch algorithm Mar 19, 2021

Implement FilterChainMatch logic

5f800e9

yashykt force-pushed the filterchainmatch branch from f3540f3 to 5f800e9 Compare March 19, 2021 11:20

yashykt added the release notes: no Indicates if PR should not be in release notes label Mar 19, 2021

yashykt marked this pull request as ready for review March 19, 2021 11:21

yashykt requested a review from markdroth as a code owner March 19, 2021 11:21

yashykt added 3 commits March 19, 2021 04:49

Add tests for transport protocol too

13d687f

Tests for duplicate NACKing

ba747d2

Introduce ConnectionManager as an interface for config fetchers

e51dfad

yashykt force-pushed the filterchainmatch branch from f367520 to e51dfad Compare March 23, 2021 00:19

yashykt added 2 commits March 23, 2021 02:46

Do not parameterize IncrementIfNonZero

115baa1

Some formatting

4832ab5

markdroth requested changes Mar 23, 2021

View reviewed changes

yashykt commented Mar 24, 2021

View reviewed changes

yashykt force-pushed the filterchainmatch branch from 0b0ba0e to 7e46e3a Compare March 24, 2021 09:33

yashykt commented Mar 24, 2021

View reviewed changes

Reviewer comments

8204e5c

yashykt force-pushed the filterchainmatch branch from 7e46e3a to 8204e5c Compare March 24, 2021 09:38

yashykt commented Mar 24, 2021

View reviewed changes

Add filter chain match information for duplicate match error

6b0bdd5

yashykt commented Mar 24, 2021

View reviewed changes

markdroth requested changes Mar 24, 2021

View reviewed changes

soheilhy reviewed Mar 24, 2021

View reviewed changes

Reviewer comments

6d1663c

yashykt commented Mar 24, 2021

View reviewed changes

yashykt force-pushed the filterchainmatch branch from 261b7ec to 6d1663c Compare March 24, 2021 20:54

Some cleanup

af133b7

markdroth requested changes Mar 24, 2021

View reviewed changes

Reviewer comments

e0b1c13

yashykt commented Mar 24, 2021

View reviewed changes

markdroth requested changes Mar 24, 2021

View reviewed changes

yashykt commented Mar 24, 2021

View reviewed changes

markdroth requested changes Mar 24, 2021

View reviewed changes

yashykt commented Mar 25, 2021

View reviewed changes

Reviewer comments

fe12acb

yashykt force-pushed the filterchainmatch branch from 743486c to fe12acb Compare March 25, 2021 11:59

yashykt commented Mar 25, 2021

View reviewed changes

markdroth requested changes Mar 25, 2021

View reviewed changes

yashykt commented Mar 25, 2021

View reviewed changes

markdroth requested changes Mar 25, 2021

View reviewed changes

Reviewer comments

48cdec5

yashykt commented Mar 25, 2021

View reviewed changes

markdroth requested changes Mar 25, 2021

View reviewed changes

yashykt commented Mar 25, 2021

View reviewed changes

markdroth approved these changes Mar 25, 2021

View reviewed changes

Clang-tidy

3f660e9

yashykt merged commit 53ba4a1 into grpc:master Mar 25, 2021

yashykt deleted the filterchainmatch branch May 18, 2023 20:30

Implement FilterChainMatch algorithm #25757

Implement FilterChainMatch algorithm #25757

Uh oh!

Conversation

yashykt commented Mar 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

markdroth left a comment

Choose a reason for hiding this comment

Uh oh!

yashykt left a comment

Choose a reason for hiding this comment

Uh oh!

yashykt left a comment

Choose a reason for hiding this comment

Uh oh!

yashykt left a comment

Choose a reason for hiding this comment

Uh oh!

yashykt left a comment

Choose a reason for hiding this comment

Uh oh!

yashykt left a comment

Choose a reason for hiding this comment

Uh oh!

markdroth left a comment

Choose a reason for hiding this comment

Uh oh!

soheilhy left a comment

Choose a reason for hiding this comment

Uh oh!

yashykt left a comment

Choose a reason for hiding this comment

Uh oh!

markdroth left a comment

Choose a reason for hiding this comment

Uh oh!

yashykt left a comment

Choose a reason for hiding this comment

Uh oh!

markdroth left a comment

Choose a reason for hiding this comment

Uh oh!

yashykt left a comment

Choose a reason for hiding this comment

Uh oh!

yashykt left a comment

Choose a reason for hiding this comment

Uh oh!

markdroth left a comment

Choose a reason for hiding this comment

Uh oh!

yashykt left a comment

Choose a reason for hiding this comment

Uh oh!

yashykt left a comment

Choose a reason for hiding this comment

Uh oh!

markdroth left a comment

Choose a reason for hiding this comment

Uh oh!

yashykt left a comment

Choose a reason for hiding this comment

Uh oh!

markdroth left a comment

Choose a reason for hiding this comment

Uh oh!

yashykt left a comment

Choose a reason for hiding this comment

Uh oh!

markdroth left a comment

Choose a reason for hiding this comment

Uh oh!

yashykt left a comment

Choose a reason for hiding this comment

Uh oh!

markdroth left a comment

Choose a reason for hiding this comment

Uh oh!

yashykt commented Mar 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

yashykt commented Mar 18, 2021 •

edited

Loading

yashykt commented Mar 25, 2021 •

edited

Loading