From: Kefu Chai Date: Mon, 19 Oct 2020 13:50:45 +0000 (+0800) Subject: common/pick_address: fail if cannot bind with specified network family X-Git-Tag: v15.2.14~10^2~7 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=0961369ba8ae5b2e1ff2c14c7771bffdb53adb2b;p=ceph.git common/pick_address: fail if cannot bind with specified network family this change partially reverts 9f75dfbf364f5140b3f291e0a2c6769bc3d8cbac we should not proceed against user's will if dual stack is specified but only one network for a network family can be found. the right fix is have better error message and documentation, not to tolerate the failure. Fixes: https://tracker.ceph.com/issues/46845 Signed-off-by: Kefu Chai (cherry picked from commit d752acafa0d99c3d7cacfaaaf3ae51770e251aff) --- diff --git a/src/common/pick_address.cc b/src/common/pick_address.cc index 32db5688b0333..ec464c1b8e165 100644 --- a/src/common/pick_address.cc +++ b/src/common/pick_address.cc @@ -282,32 +282,6 @@ static int fill_in_one_address( return 0; } -unsigned networks_address_family_coverage(CephContext *cct, const std::string &networks) { - std::list nets; - get_str_list(networks, nets); - unsigned found_ipv = 0; - - for (auto& s : nets) { - struct sockaddr_storage net; - unsigned prefix_len; - if (!parse_network(s.c_str(), &net, &prefix_len)) { - lderr(cct) << "unable to parse network: " << s << dendl; - exit(1); - } - - switch (net.ss_family) { - case AF_INET: - found_ipv |= CEPH_PICK_ADDRESS_IPV4; - break; - case AF_INET6: - found_ipv |= CEPH_PICK_ADDRESS_IPV6; - break; - } - } - - return found_ipv; -} - int pick_addresses( CephContext *cct, unsigned flags, @@ -381,7 +355,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; - unsigned found_ipv = networks_address_family_coverage(cct, networks); // first try on preferred numa node (if >= 0), then anywhere. while (true) { // note: pass in ipv to filter the matching addresses @@ -402,11 +375,6 @@ int pick_addresses( networks, interfaces, addrs, preferred_numa_node); } - if (found_ipv != 0 && (found_ipv & ipv != ipv)) { - lderr(cct) << "An IP address was found, but not enough networks to cover both address families. " - << "An IPv4 and IPv6 network is required for dual stack. Continuing with one stack" << dendl; - break; - } if (ipv4_r >= 0 && ipv6_r >= 0) { break; } diff --git a/src/common/pick_address.h b/src/common/pick_address.h index 2ae67e86ea6b0..2621fed8108db 100644 --- a/src/common/pick_address.h +++ b/src/common/pick_address.h @@ -80,11 +80,4 @@ int get_iface_numa_node( const std::string& iface, int *node); -/** - * Return a bitmap of address families that are covered by networks - * - * @param cct context (used for logging) - * @param string of networks - */ -unsigned networks_address_family_coverage(CephContext *cct, const std::string &networks); #endif diff --git a/src/test/test_ipaddr.cc b/src/test/test_ipaddr.cc index 6f6c39fcb2b25..d904a6819fffd 100644 --- a/src/test/test_ipaddr.cc +++ b/src/test/test_ipaddr.cc @@ -931,26 +931,20 @@ TEST(pick_address, filtering) TEST(pick_address, ipv4_ipv6_enabled) { - struct ifaddrs one, two; + struct ifaddrs one; struct sockaddr_in a_one; - struct sockaddr_in6 a_two; - one.ifa_next = &two; + one.ifa_next = NULL; one.ifa_addr = (struct sockaddr*)&a_one; one.ifa_name = eth0; - two.ifa_next = NULL; - two.ifa_addr = (struct sockaddr*)&a_two; - two.ifa_name = eth0; - ipv4(&a_one, "10.1.1.2"); - ipv6(&a_two, "2001:1234:5678:90ab::cdef"); CephContext *cct = new CephContext(CEPH_ENTITY_TYPE_OSD); cct->_conf._clear_safe_to_start_threads(); // so we can set configs cct->_conf.set_val("public_addr", ""); - cct->_conf.set_val("public_network", "10.1.1.0/24, 2001::/16"); + cct->_conf.set_val("public_network", "10.1.1.0/24"); cct->_conf.set_val("public_network_interface", ""); cct->_conf.set_val("cluster_addr", ""); cct->_conf.set_val("cluster_network", ""); @@ -964,15 +958,11 @@ TEST(pick_address, ipv4_ipv6_enabled) CEPH_PICK_ADDRESS_MSGR1, &one, &av); cout << av << std::endl; - ASSERT_EQ(0, r); - // Got 2 address - ASSERT_EQ(2u, av.v.size()); - ASSERT_EQ(string("v1:[2001:1234:5678:90ab::cdef]:0/0"), stringify(av.v[0])); - ASSERT_EQ(string("v1:10.1.1.2:0/0"), stringify(av.v[1])); + ASSERT_EQ(-1, r); } } -TEST(pick_address, only_ipv6_enabled) +TEST(pick_address, ipv4_ipv6_enabled2) { struct ifaddrs one; struct sockaddr_in6 a_one; @@ -993,76 +983,6 @@ TEST(pick_address, only_ipv6_enabled) cct->_conf.set_val("cluster_network", ""); cct->_conf.set_val("cluster_network_interface", ""); cct->_conf.set_val("ms_bind_ipv6", "true"); - cct->_conf.set_val("ms_bind_ipv4", "false"); - - entity_addrvec_t av; - { - int r = pick_addresses(cct, - CEPH_PICK_ADDRESS_PUBLIC | - CEPH_PICK_ADDRESS_MSGR1, - &one, &av); - cout << av << std::endl; - ASSERT_EQ(0, r); - ASSERT_EQ(1u, av.v.size()); - ASSERT_EQ(string("v1:[2001:1234:5678:90ab::cdef]:0/0"), stringify(av.v[0])); - } -} - -TEST(pick_address, only_ipv4_enabled) -{ - struct ifaddrs one; - struct sockaddr_in a_one; - - one.ifa_next = NULL; - one.ifa_addr = (struct sockaddr*)&a_one; - one.ifa_name = eth0; - - ipv4(&a_one, "10.1.1.2"); - - CephContext *cct = new CephContext(CEPH_ENTITY_TYPE_OSD); - cct->_conf._clear_safe_to_start_threads(); // so we can set configs - - cct->_conf.set_val("public_addr", ""); - cct->_conf.set_val("public_network", "10.1.1.0/24"); - cct->_conf.set_val("public_network_interface", ""); - cct->_conf.set_val("cluster_addr", ""); - cct->_conf.set_val("cluster_network", ""); - cct->_conf.set_val("cluster_network_interface", ""); - - entity_addrvec_t av; - { - int r = pick_addresses(cct, - CEPH_PICK_ADDRESS_PUBLIC | - CEPH_PICK_ADDRESS_MSGR1, - &one, &av); - cout << av << std::endl; - ASSERT_EQ(0, r); - ASSERT_EQ(1u, av.v.size()); - ASSERT_EQ(string("v1:10.1.1.2:0/0"), stringify(av.v[0])); - } -} - -TEST(pick_address, ipv4_ipv6_enabled_not_enough_networks) -{ - struct ifaddrs one; - struct sockaddr_in a_one; - - one.ifa_next = NULL; - one.ifa_addr = (struct sockaddr*)&a_one; - one.ifa_name = eth0; - - ipv4(&a_one, "10.1.1.2"); - - CephContext *cct = new CephContext(CEPH_ENTITY_TYPE_OSD); - cct->_conf._clear_safe_to_start_threads(); // so we can set configs - - cct->_conf.set_val("public_addr", ""); - cct->_conf.set_val("public_network", "10.1.1.0/24"); - cct->_conf.set_val("public_network_interface", ""); - cct->_conf.set_val("cluster_addr", ""); - cct->_conf.set_val("cluster_network", ""); - cct->_conf.set_val("cluster_network_interface", ""); - cct->_conf.set_val("ms_bind_ipv6", "true"); entity_addrvec_t av; { @@ -1072,31 +992,6 @@ TEST(pick_address, ipv4_ipv6_enabled_not_enough_networks) &one, &av); cout << av << std::endl; ASSERT_EQ(-1, r); - ASSERT_EQ(1u, av.v.size()); - ASSERT_EQ(string("v2:10.1.1.2:0/0"), stringify(av.v[0])); } } -TEST(networks_address_family_coverage, just_ipv4) -{ - CephContext *cct = new CephContext(CEPH_ENTITY_TYPE_OSD); - std::string networks = "10.0.0.0/24"; - unsigned r = networks_address_family_coverage(cct, networks); - ASSERT_EQ(CEPH_PICK_ADDRESS_IPV4, r); -} - -TEST(networks_address_family_coverage, just_ipv6) -{ - CephContext *cct = new CephContext(CEPH_ENTITY_TYPE_OSD); - std::string networks = "2001::/16"; - unsigned r = networks_address_family_coverage(cct, networks); - ASSERT_EQ(CEPH_PICK_ADDRESS_IPV6, r); -} - -TEST(networks_address_family_coverage, ipv6_and_ipv4) -{ - CephContext *cct = new CephContext(CEPH_ENTITY_TYPE_OSD); - std::string networks = "2001::/16, 10.0.0.0/16"; - unsigned r = networks_address_family_coverage(cct, networks); - ASSERT_EQ(CEPH_PICK_ADDRESS_IPV4 | CEPH_PICK_ADDRESS_IPV6, r); -}