From: Kefu Chai Date: Wed, 21 Apr 2021 11:50:48 +0000 (+0800) Subject: common/pick_address: prefer non-loopback addresses X-Git-Tag: v17.1.0~2103^2~2 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=a9b9bcd53215a07608a28ac2c8e4a8c8b8e80e66;p=ceph.git common/pick_address: prefer non-loopback addresses instead of filtering out loopback ifaces, check for loopback addresses, and prefer non-loopback addresses over loopback addresses. before this change, iface named "lo" is filtered out by default, and "lo" is allowed if `ms_bind_exclude_lo_iface` is false. after this change, iface with address out of 127/8 is prefered. the iface marked down is not considered. the option of "ms_bind_exclude_lo_iface" is removed. the tests are updated accordingly. Fixes: https://tracker.ceph.com/issues/50456 Signed-off-by: Kefu Chai --- diff --git a/src/common/options/global.yaml.in b/src/common/options/global.yaml.in index ebb9ca3fe7d98..5849b9b7e6e61 100644 --- a/src/common/options/global.yaml.in +++ b/src/common/options/global.yaml.in @@ -1173,13 +1173,6 @@ options: fmt_desc: Throttles total size of messages waiting to be dispatched. default: 100_M with_legacy: true -- name: ms_bind_exclude_lo_iface - type: bool - level: advanced - desc: Allow servers to bind loopback network interfaces (lo) - default: true - flags: - - startup - name: ms_bind_ipv4 type: bool level: advanced diff --git a/src/common/pick_address.cc b/src/common/pick_address.cc index 47f8875e0cedf..45066a836cdec 100644 --- a/src/common/pick_address.cc +++ b/src/common/pick_address.cc @@ -15,6 +15,7 @@ #include "common/pick_address.h" #include +#include #include #include #include @@ -45,15 +46,42 @@ bool matches_with_name(const ifaddrs& ifa, const std::string& if_name) return if_name.compare(ifa.ifa_name) == 0; } -bool is_addr_allowed(const ifaddrs& ifa, bool exclude_lo_iface) +static int is_loopback_addr(sockaddr* addr) { - if (exclude_lo_iface && strcmp(ifa.ifa_name, "lo") == 0) { - return false; + if (addr->sa_family == AF_INET) { + const sockaddr_in* sin = (struct sockaddr_in *)(addr); + const in_addr_t net = ntohl(sin->sin_addr.s_addr) >> IN_CLASSA_NSHIFT; + return net == IN_LOOPBACKNET ? 1 : 0; + } else if (addr->sa_family == AF_INET6) { + sockaddr_in6* sin6 = (struct sockaddr_in6 *)(addr); + return IN6_IS_ADDR_LOOPBACK(&sin6->sin6_addr) ? 1 : 0; + } else { + return -1; } - if (boost::starts_with(ifa.ifa_name, "lo:")) { - return false; +} + +static int grade_addr(const ifaddrs& ifa) +{ + if (ifa.ifa_addr == nullptr) { + return -1; } - return true; + int score = 0; + if (ifa.ifa_flags & IFF_UP) { + score += 4; + } + switch (is_loopback_addr(ifa.ifa_addr)) { + case 0: + // prefer non-loopback addresses + score += 2; + break; + case 1: + score += 0; + break; + default: + score = -1; + break; + } + return score; } bool matches_with_net(const ifaddrs& ifa, @@ -107,13 +135,13 @@ bool matches_with_numa_node(const ifaddrs& ifa, int numa_node) #endif } } + const struct sockaddr *find_ip_in_subnet_list( CephContext *cct, const struct ifaddrs *ifa, unsigned ipv, const std::string &networks, const std::string &interfaces, - bool exclude_lo_iface, int numa_node) { const auto ifs = get_str_list(interfaces); @@ -123,25 +151,32 @@ const struct sockaddr *find_ip_in_subnet_list( exit(1); } + int best_score = -1; + const sockaddr* best_addr = nullptr; for (const auto* addr = ifa; addr != nullptr; addr = addr->ifa_next) { - if (!is_addr_allowed(*addr, exclude_lo_iface)) { - continue; - } - if ((ifs.empty() || - std::any_of(std::begin(ifs), std::end(ifs), + if (!ifs.empty() && + std::none_of(std::begin(ifs), std::end(ifs), [&](const auto& if_name) { return matches_with_name(*addr, if_name); - })) && - (nets.empty() || - std::any_of(std::begin(nets), std::end(nets), + })) { + continue; + } + if (!nets.empty() && + std::none_of(std::begin(nets), std::end(nets), [&](const auto& net) { return matches_with_net(cct, *addr, net, ipv); - })) && - matches_with_numa_node(*addr, numa_node)) { - return addr->ifa_addr; + })) { + continue; + } + if (!matches_with_numa_node(*addr, numa_node)) { + continue; + } + if (int score = grade_addr(*addr); score > best_score) { + best_score = score; + best_addr = addr->ifa_addr; } } - return nullptr; + return best_addr; } #ifndef WITH_SEASTAR @@ -166,7 +201,6 @@ static void fill_in_one_address(CephContext *cct, const struct ifaddrs *ifa, const string &networks, const string &interfaces, - bool exclude_lo_iface, const char *conf_var, int numa_node = -1) { @@ -176,7 +210,6 @@ static void fill_in_one_address(CephContext *cct, CEPH_PICK_ADDRESS_IPV4|CEPH_PICK_ADDRESS_IPV6, networks, interfaces, - exclude_lo_iface, numa_node); if (!found) { lderr(cct) << "unable to find any IP address in networks '" << networks @@ -229,27 +262,21 @@ void pick_addresses(CephContext *cct, int needs) exit(1); } auto free_ifa = make_scope_guard([ifa] { freeifaddrs(ifa); }); - - const bool exclude_lo_iface = - cct->_conf.get_val("ms_bind_exclude_lo_iface"); if ((needs & CEPH_PICK_ADDRESS_PUBLIC) && public_addr.is_blank_ip() && !public_network.empty()) { fill_in_one_address(cct, ifa, public_network, public_network_interface, - exclude_lo_iface, "public_addr"); } if ((needs & CEPH_PICK_ADDRESS_CLUSTER) && cluster_addr.is_blank_ip()) { if (!cluster_network.empty()) { fill_in_one_address(cct, ifa, cluster_network, cluster_network_interface, - exclude_lo_iface, "cluster_addr"); } else { if (!public_network.empty()) { lderr(cct) << "Public network was set, but cluster network was not set " << dendl; lderr(cct) << " Using public network also for cluster network" << dendl; fill_in_one_address(cct, ifa, public_network, public_network_interface, - exclude_lo_iface, "cluster_addr"); } } @@ -263,14 +290,12 @@ static int fill_in_one_address( unsigned ipv, const string &networks, const string &interfaces, - bool exclude_lo_iface, entity_addrvec_t *addrs, int numa_node = -1) { const struct sockaddr *found = find_ip_in_subnet_list(cct, ifa, ipv, networks, interfaces, - exclude_lo_iface, numa_node); if (!found) { std::string ip_type = ""; @@ -384,8 +409,6 @@ int pick_addresses( !networks.empty()) { int ipv4_r = !(ipv & CEPH_PICK_ADDRESS_IPV4) ? 0 : -1; int ipv6_r = !(ipv & CEPH_PICK_ADDRESS_IPV6) ? 0 : -1; - const bool exclude_lo_iface = - cct->_conf.get_val("ms_bind_exclude_lo_iface"); // first try on preferred numa node (if >= 0), then anywhere. while (true) { // note: pass in ipv to filter the matching addresses @@ -393,14 +416,12 @@ int pick_addresses( (flags & CEPH_PICK_ADDRESS_PREFER_IPV4)) { ipv4_r = fill_in_one_address(cct, ifa, CEPH_PICK_ADDRESS_IPV4, networks, interfaces, - exclude_lo_iface, addrs, preferred_numa_node); } if (ipv & CEPH_PICK_ADDRESS_IPV6) { ipv6_r = fill_in_one_address(cct, ifa, CEPH_PICK_ADDRESS_IPV6, networks, interfaces, - exclude_lo_iface, addrs, preferred_numa_node); } @@ -408,7 +429,6 @@ int pick_addresses( !(flags & CEPH_PICK_ADDRESS_PREFER_IPV4)) { ipv4_r = fill_in_one_address(cct, ifa, CEPH_PICK_ADDRESS_IPV4, networks, interfaces, - exclude_lo_iface, addrs, preferred_numa_node); } diff --git a/src/common/pick_address.h b/src/common/pick_address.h index 2a41f5a8d03c9..1faf60dbd75bc 100644 --- a/src/common/pick_address.h +++ b/src/common/pick_address.h @@ -88,7 +88,6 @@ const struct sockaddr *find_ip_in_subnet_list( unsigned ipv, const std::string &networks, const std::string &interfaces, - bool exclude_lo_iface, int numa_node=-1); int get_iface_numa_node( diff --git a/src/test/test_ipaddr.cc b/src/test/test_ipaddr.cc index d2ab8c50eeed3..def7df0b8c6f1 100644 --- a/src/test/test_ipaddr.cc +++ b/src/test/test_ipaddr.cc @@ -13,6 +13,7 @@ #endif #include #include +#include static void ipv4(struct sockaddr_in *addr, const char *s) { int err; @@ -190,10 +191,12 @@ TEST(CommonIPAddr, TestV4_SkipLoopback) one.ifa_name = lo; two.ifa_next = &three; + two.ifa_flags = IFF_UP; two.ifa_addr = (struct sockaddr*)&a_two; two.ifa_name = lo0; three.ifa_next = NULL; + three.ifa_flags = IFF_UP; three.ifa_addr = (struct sockaddr*)&a_three; three.ifa_name = eth0; @@ -202,21 +205,18 @@ TEST(CommonIPAddr, TestV4_SkipLoopback) ipv4(&a_three, "10.1.2.3"); const struct sockaddr *result = nullptr; + // we prefer the non-loopback address despite the loopback addresses result = find_ip_in_subnet_list(nullptr, (struct ifaddrs*)&one, CEPH_PICK_ADDRESS_IPV4 | CEPH_PICK_ADDRESS_IPV6, - "10.0.0.0/8", "", true); + "", ""); ASSERT_EQ((struct sockaddr*)&a_three, result); + // the subnet criteria leaves us no choice but the UP loopback address result = find_ip_in_subnet_list(nullptr, (struct ifaddrs*)&one, CEPH_PICK_ADDRESS_IPV4 | CEPH_PICK_ADDRESS_IPV6, - "127.0.0.0/8", "", true); - ASSERT_EQ(nullptr, result); - result = - find_ip_in_subnet_list(nullptr, (struct ifaddrs*)&one, - CEPH_PICK_ADDRESS_IPV4 | CEPH_PICK_ADDRESS_IPV6, - "127.0.0.0/8", "", false); - ASSERT_EQ((struct sockaddr*)&a_one, result); + "127.0.0.0/8", ""); + ASSERT_EQ((struct sockaddr*)&a_two, result); } TEST(CommonIPAddr, TestV6_Simple) @@ -316,43 +316,37 @@ TEST(CommonIPAddr, TestV6_SkipLoopback) struct sockaddr_in6 a_one; struct sockaddr_in6 a_two; struct sockaddr_in6 a_three; - struct sockaddr_in6 net; - - memset(&net, 0, sizeof(net)); one.ifa_next = &two; + ipv6(&a_one, "::1"); one.ifa_addr = (struct sockaddr*)&a_one; one.ifa_name = lo; two.ifa_next = &three; + two.ifa_flags = IFF_UP; + ipv6(&a_two, "::1"); two.ifa_addr = (struct sockaddr*)&a_two; two.ifa_name = lo0; three.ifa_next = NULL; + three.ifa_flags = IFF_UP; + ipv6(&a_three, "2001:1234:5678:90ab::beef"); three.ifa_addr = (struct sockaddr*)&a_three; three.ifa_name = eth0; - ipv6(&a_one, "::1"); - ipv6(&a_two, "::1"); - ipv6(&a_three, "2001:1234:5678:90ab::beef"); - ipv6(&net, "ff00::1"); - const struct sockaddr *result = nullptr; + // we prefer the non-loopback address despite the loopback addresses result = find_ip_in_subnet_list(nullptr, (struct ifaddrs*)&one, CEPH_PICK_ADDRESS_IPV4 | CEPH_PICK_ADDRESS_IPV6, - "2001:1234:5678:90ab::0/64", "", true); + "", ""); ASSERT_EQ((struct sockaddr*)&a_three, result); + // the subnet criteria leaves us no choice but the UP loopback address result = find_ip_in_subnet_list(nullptr, (struct ifaddrs*)&one, CEPH_PICK_ADDRESS_IPV4 | CEPH_PICK_ADDRESS_IPV6, - "::1/128", "", true); - ASSERT_EQ(nullptr, result); - result = - find_ip_in_subnet_list(nullptr, (struct ifaddrs*)&one, - CEPH_PICK_ADDRESS_IPV4 | CEPH_PICK_ADDRESS_IPV6, - "::1/128", "", false); - ASSERT_EQ((struct sockaddr*)&a_one, result); + "::1/128", ""); + ASSERT_EQ((struct sockaddr*)&a_two, result); } TEST(CommonIPAddr, ParseNetwork_Empty) @@ -709,8 +703,7 @@ TEST(pick_address, find_ip_in_subnet_list) &one, CEPH_PICK_ADDRESS_IPV4, "10.1.0.0/16", - "eth0", - true); + "eth0"); ASSERT_EQ((struct sockaddr*)&a_one, result); result = find_ip_in_subnet_list( @@ -718,8 +711,7 @@ TEST(pick_address, find_ip_in_subnet_list) &one, CEPH_PICK_ADDRESS_IPV4, "10.2.0.0/16", - "eth1", - true); + "eth1"); ASSERT_EQ((struct sockaddr*)&a_two, result); // match by eth name @@ -728,8 +720,7 @@ TEST(pick_address, find_ip_in_subnet_list) &one, CEPH_PICK_ADDRESS_IPV4, "10.0.0.0/8", - "eth0", - true); + "eth0"); ASSERT_EQ((struct sockaddr*)&a_one, result); result = find_ip_in_subnet_list( @@ -737,8 +728,7 @@ TEST(pick_address, find_ip_in_subnet_list) &one, CEPH_PICK_ADDRESS_IPV4, "10.0.0.0/8", - "eth1", - true); + "eth1"); ASSERT_EQ((struct sockaddr*)&a_two, result); result = find_ip_in_subnet_list( @@ -746,8 +736,7 @@ TEST(pick_address, find_ip_in_subnet_list) &one, CEPH_PICK_ADDRESS_IPV6, "2001::/16", - "eth1", - true); + "eth1"); ASSERT_EQ((struct sockaddr*)&a_three, result); }