From: Sage Weil Date: Fri, 25 Jan 2019 21:14:56 +0000 (-0600) Subject: msg/async: do not use peer to addr detection; use getsockname() X-Git-Tag: v14.1.0~271^2~6 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=98a9a5e5386294ade5543da076892975d079afe9;p=ceph-ci.git msg/async: do not use peer to addr detection; use getsockname() If of relying on the peer to tell us what address we are connecting from, look at how our local socket is bound, and use that address. This removes the possibility for error because we will infer our address locally and that will be the one place it is decide; the server will just use our value. As things were previously, we had to make the local and remote inference match, which was fragile. This does take away the client's ability to discover if it is traversing NAT to reach the server and learning its public/external address. I don't think anybody has ever tested this, so it probably didn't even work, and I've never heard it come up as a requirement. Signed-off-by: Sage Weil --- diff --git a/src/msg/async/ProtocolV2.cc b/src/msg/async/ProtocolV2.cc index e87b202f2a6..d6e9f125738 100644 --- a/src/msg/async/ProtocolV2.cc +++ b/src/msg/async/ProtocolV2.cc @@ -2197,24 +2197,45 @@ CtPtr ProtocolV2::send_client_ident() { flags |= CEPH_MSG_CONNECT_LOSSY; } - entity_addrvec_t ma = messenger->get_myaddrs(); - if (ma.empty()) { + if (messenger->get_myaddrs().empty() || + messenger->get_myaddrs().front().is_blank_ip()) { + sockaddr_storage ss; + socklen_t len = sizeof(ss); + getsockname(connection->cs.fd(), (sockaddr*)&ss, &len); + ldout(cct,1) << __func__ << " getsockname reveals I am " << (sockaddr*)&ss + << " when talking to " << connection->target_addr << dendl; entity_addr_t a; - if (connection->target_addr.is_ipv6()) { - a.parse("[::]:0"); - } else { - a.parse("0.0.0.0:0"); + a.set_type(connection->target_addr.get_type()); + a.set_sockaddr((sockaddr*)&ss); + a.set_port(0); + connection->lock.unlock(); + messenger->learned_addr(a); + if (cct->_conf->ms_inject_internal_delays && + cct->_conf->ms_inject_socket_failures) { + if (rand() % cct->_conf->ms_inject_socket_failures == 0) { + ldout(cct, 10) << __func__ << " sleep for " + << cct->_conf->ms_inject_internal_delays << dendl; + utime_t t; + t.set_from_double(cct->_conf->ms_inject_internal_delays); + t.sleep(); + } + } + connection->lock.lock(); + if (state != CONNECTING) { + ldout(cct, 1) << __func__ + << " state changed while learned_addr, mark_down or " + << " replacing must be happened just now" << dendl; + return nullptr; } - a.set_nonce(messenger->get_nonce()); - ma.v.push_back(a); } - ClientIdentFrame client_ident(this, ma, + + ClientIdentFrame client_ident(this, messenger->get_myaddrs(), messenger->get_myname().num(), global_seq, connection->policy.features_supported, connection->policy.features_required, flags); ldout(cct, 5) << __func__ << " sending identification: " - << "addrs=" << ma + << "addrs=" << messenger->get_myaddrs() << " gid=" << messenger->get_myname().num() << " global_seq=" << global_seq << " features_supported=" << std::hex @@ -2342,26 +2363,6 @@ CtPtr ProtocolV2::handle_server_ident(char *payload, uint32_t length) { return _fault(); } - connection->lock.unlock(); - messenger->learned_addr(server_ident.peer_addr()); - if (cct->_conf->ms_inject_internal_delays && - cct->_conf->ms_inject_socket_failures) { - if (rand() % cct->_conf->ms_inject_socket_failures == 0) { - ldout(cct, 10) << __func__ << " sleep for " - << cct->_conf->ms_inject_internal_delays << dendl; - utime_t t; - t.set_from_double(cct->_conf->ms_inject_internal_delays); - t.sleep(); - } - } - connection->lock.lock(); - if (state != CONNECTING) { - ldout(cct, 1) << __func__ - << " state changed while learned_addr, mark_down or " - << " replacing must be happened just now" << dendl; - return nullptr; - } - cookie = server_ident.cookie(); connection->set_peer_addrs(server_ident.addrs()); @@ -2561,41 +2562,13 @@ CtPtr ProtocolV2::handle_client_ident(char *payload, uint32_t length) { << client_ident.supported_features() << " features_required=" << client_ident.required_features() << " flags=" << client_ident.flags() << std::dec << dendl; - - connection->target_addr.set_type(entity_addr_t::TYPE_MSGR2); - - if (client_ident.addrs().empty()) { + if (client_ident.addrs().empty() || + !client_ident.addrs().has_msgr2()) { return _fault(); // a v2 peer should never do this - } else { - for (auto &peer_addr : client_ident.addrs().v) { - if (peer_addr.is_blank_ip()) { - // peer apparently doesn't know what ip they have; figure it out for - // them. - int port = peer_addr.get_port(); - peer_addr.u = connection->target_addr.u; - peer_addr.set_port(port); - ldout(cct, 0) << __func__ << " peer addr is really " << peer_addr - << " (socket is " << connection->target_addr << ")" - << dendl; - } - } - - entity_addr_t peer_addr = client_ident.addrs().msgr2_addr(); - if (peer_addr.type == entity_addr_t::TYPE_NONE) { - // no v2 address is known - peer_addr = client_ident.addrs().legacy_addr(); - peer_addr.set_type(entity_addr_t::TYPE_MSGR2); - entity_addrvec_t addrs; - addrs.v.push_back(peer_addr); - for (const auto &addr : client_ident.addrs().v) { - addrs.v.push_back(addr); - } - connection->set_peer_addrs(addrs); - } else { - connection->set_peer_addrs(client_ident.addrs()); - } - connection->target_addr = peer_addr; } + connection->set_peer_addrs(client_ident.addrs()); + connection->target_addr = client_ident.addrs().msgr2_addr(); + peer_name = entity_name_t(connection->get_peer_type(), client_ident.gid()); uint64_t feat_missing = connection->policy.features_required &